-
-
Notifications
You must be signed in to change notification settings - Fork 99
Fix v0.16 codemod: add shadow checks to transformUseFetch #3836
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from 3 commits
Commits
Show all changes
17 commits
Select commit
Hold shift + click to select a range
97381ef
docs: Add v0.16 migration codemod
ntucker e808683
fix: bugbot
ntucker 8d10823
Fix schema codemod: aliased import skipped when name already exists i…
cursoragent 42be09c
fix: scope useFetch truthiness rewrites to enclosing function
cursoragent faf1adc
fix(codemod): use optional chaining in useFetch truthiness rewrite
cursoragent 019edc8
fix(codemod): rewrite useFetch vars on both sides of LogicalExpression
cursoragent ca2a692
fix(codemod): preserve schema import when bare identifier references …
cursoragent c7d3c25
fix: bugbot
ntucker 460f3da
fix(codemod): skip schema.Object and schema.Array in v0.16 transform
cursoragent a7677f3
Fix transformUseFetch to only apply when useFetch is imported from @d…
cursoragent 255f8f2
fix(codemod): skip schema.X rewrites where schema is shadowed by loca…
cursoragent 94727d0
fix(codemod): preserve boolean semantics for useFetch conditional fet…
cursoragent b9c004e
fix(codemod): detect top-level var/function/class bindings in scopeBi…
cursoragent 9b24df8
fix(codemod): detect FunctionDeclaration/ClassDeclaration shadowing i…
cursoragent e57ce92
Fix transformUseFetch rewriting shadowed variables in nested callbacks
cursoragent 460c9de
Guard transformPaths with @data-client import check
cursoragent b73054e
Fix resolveLocal collision: verify 'Schema'+name is also free of conf…
cursoragent File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,258 @@ | ||
| 'use strict'; | ||
|
|
||
| /** | ||
| * @data-client v0.16 migration codemod | ||
| * | ||
| * Transforms: | ||
| * 1. path-to-regexp v6 → v8 syntax in RestEndpoint/resource path strings | ||
| * 2. useFetch() truthiness checks → .resolved checks | ||
| * 3. schema.X namespace → direct imports | ||
| * | ||
| * Usage: | ||
| * npx jscodeshift -t https://dataclient.io/codemods/v0.16.js --extensions=ts,tsx,js,jsx src/ | ||
| */ | ||
|
|
||
| const DATA_CLIENT_PACKAGES = new Set([ | ||
| '@data-client/endpoint', | ||
| '@data-client/rest', | ||
| ]); | ||
|
|
||
| // --- path-to-regexp v6 → v8 --- | ||
|
|
||
| function transformPathString(s) { | ||
| // :name(\d+) → :name (handles nested non-capturing groups) | ||
| s = s.replace(/:(\w+)\((?:[^()]*|\([^()]*\))*\)/g, ':$1'); | ||
| // {group}? → {group} | ||
| s = s.replace(/(\{[^}]+\})\?/g, '$1'); | ||
| // /:name? → {/:name} (also handles - . ~ prefixes) | ||
| s = s.replace(/([/\-.~]):(\w+)\?/g, '{$1:$2}'); | ||
| // /:name+ → /*name | ||
| s = s.replace(/\/:(\w+)\+/g, '/*$1'); | ||
| // /:name* → {/*name} | ||
| s = s.replace(/\/:(\w+)\*/g, '{/*$1}'); | ||
| // \? → ? and \+ → + (no longer special in v8) | ||
| s = s.replace(/\\([?+])/g, '$1'); | ||
| return s; | ||
| } | ||
|
|
||
| function transformPaths(j, root) { | ||
| let dirty = false; | ||
|
|
||
| root.find(j.StringLiteral).forEach(path => { | ||
| const parent = path.parent.node; | ||
| if ( | ||
| (parent.type === 'Property' || parent.type === 'ObjectProperty') && | ||
| parent.value === path.node && | ||
| parent.key && | ||
| (parent.key.name === 'path' || | ||
| (parent.key.type === 'StringLiteral' && parent.key.value === 'path')) | ||
| ) { | ||
| const updated = transformPathString(path.node.value); | ||
| if (updated !== path.node.value) { | ||
| path.node.value = updated; | ||
| if (path.node.extra) { | ||
| delete path.node.extra.raw; | ||
| delete path.node.extra.rawValue; | ||
| } | ||
| dirty = true; | ||
| } | ||
| } | ||
| }); | ||
|
|
||
| return dirty; | ||
| } | ||
|
|
||
| // --- useFetch() truthiness → .resolved --- | ||
|
|
||
| function transformUseFetch(j, root) { | ||
| let dirty = false; | ||
| const vars = new Set(); | ||
|
|
||
| root.find(j.VariableDeclarator).forEach(p => { | ||
| const init = p.node.init; | ||
| if ( | ||
| init && | ||
| init.type === 'CallExpression' && | ||
| init.callee && | ||
| init.callee.type === 'Identifier' && | ||
| init.callee.name === 'useFetch' && | ||
| p.node.id.type === 'Identifier' | ||
| ) { | ||
| vars.add(p.node.id.name); | ||
| } | ||
| }); | ||
|
|
||
| if (!vars.size) return dirty; | ||
|
|
||
| function rewrite(test) { | ||
| // promise → !promise.resolved | ||
| if (test.type === 'Identifier' && vars.has(test.name)) { | ||
| return j.unaryExpression( | ||
| '!', | ||
| j.memberExpression(j.identifier(test.name), j.identifier('resolved')), | ||
| ); | ||
|
cursor[bot] marked this conversation as resolved.
|
||
| } | ||
| // !promise → promise.resolved | ||
| if ( | ||
| test.type === 'UnaryExpression' && | ||
| test.operator === '!' && | ||
| test.argument.type === 'Identifier' && | ||
| vars.has(test.argument.name) | ||
| ) { | ||
| return j.memberExpression( | ||
| j.identifier(test.argument.name), | ||
| j.identifier('resolved'), | ||
| ); | ||
| } | ||
|
cursor[bot] marked this conversation as resolved.
|
||
| return null; | ||
| } | ||
|
|
||
| [j.IfStatement, j.ConditionalExpression].forEach(type => { | ||
| root.find(type).forEach(p => { | ||
| const r = rewrite(p.node.test); | ||
| if (r) { | ||
| p.node.test = r; | ||
| dirty = true; | ||
| } | ||
| }); | ||
| }); | ||
|
|
||
| root.find(j.LogicalExpression, { operator: '&&' }).forEach(p => { | ||
| const r = rewrite(p.node.left); | ||
| if (r) { | ||
| p.node.left = r; | ||
| dirty = true; | ||
| } | ||
| }); | ||
|
cursor[bot] marked this conversation as resolved.
|
||
|
|
||
|
cursor[bot] marked this conversation as resolved.
|
||
| return dirty; | ||
| } | ||
|
|
||
| // --- schema.X namespace → direct imports --- | ||
|
|
||
| const JS_GLOBALS = new Set([ | ||
| 'Array', | ||
| 'Boolean', | ||
| 'Date', | ||
| 'Error', | ||
| 'Function', | ||
| 'JSON', | ||
| 'Map', | ||
| 'Math', | ||
| 'Number', | ||
| 'Object', | ||
| 'Promise', | ||
| 'Proxy', | ||
| 'Reflect', | ||
| 'RegExp', | ||
| 'Set', | ||
| 'String', | ||
| 'Symbol', | ||
| 'WeakMap', | ||
| 'WeakSet', | ||
| ]); | ||
|
|
||
| function transformSchemaImports(j, root) { | ||
| let dirty = false; | ||
|
|
||
| root.find(j.ImportDeclaration).forEach(importPath => { | ||
| const source = importPath.node.source.value; | ||
| if (!DATA_CLIENT_PACKAGES.has(source)) return; | ||
|
|
||
| const specs = importPath.node.specifiers; | ||
| const idx = specs.findIndex( | ||
| s => | ||
| s.type === 'ImportSpecifier' && | ||
| s.imported && | ||
| (s.imported.name === 'schema' || s.imported.value === 'schema'), | ||
| ); | ||
| if (idx === -1) return; | ||
|
|
||
| const local = specs[idx].local.name; | ||
| // Map from exported name → local identifier to use in code | ||
| const used = new Map(); | ||
|
|
||
| const scopeBindings = new Set(); | ||
| root.find(j.ImportDeclaration).forEach(ip => { | ||
| if (ip === importPath) return; | ||
| ip.node.specifiers.forEach(s => { | ||
| if (s.local) scopeBindings.add(s.local.name); | ||
| }); | ||
| }); | ||
|
ntucker marked this conversation as resolved.
|
||
|
|
||
| function resolveLocal(name) { | ||
| if (JS_GLOBALS.has(name) || scopeBindings.has(name)) { | ||
| return 'Schema' + name; | ||
| } | ||
| return name; | ||
| } | ||
|
cursor[bot] marked this conversation as resolved.
|
||
|
|
||
| root | ||
| .find(j.MemberExpression, { | ||
| object: { type: 'Identifier', name: local }, | ||
| }) | ||
| .forEach(mp => { | ||
| if (mp.node.property.type === 'Identifier') { | ||
| const name = mp.node.property.name; | ||
| if (!used.has(name)) used.set(name, resolveLocal(name)); | ||
| j(mp).replaceWith(j.identifier(used.get(name))); | ||
| } | ||
| }); | ||
|
cursor[bot] marked this conversation as resolved.
cursor[bot] marked this conversation as resolved.
cursor[bot] marked this conversation as resolved.
|
||
|
|
||
| // TypeScript qualified names: schema.Union in type positions | ||
| try { | ||
| root | ||
| .find(j.TSQualifiedName, { | ||
| left: { type: 'Identifier', name: local }, | ||
| }) | ||
| .forEach(qp => { | ||
| if (qp.node.right.type === 'Identifier') { | ||
| const name = qp.node.right.name; | ||
| if (!used.has(name)) used.set(name, resolveLocal(name)); | ||
| j(qp).replaceWith(j.identifier(used.get(name))); | ||
| } | ||
| }); | ||
| } catch (_) {} | ||
|
|
||
| if (!used.size) return; | ||
|
|
||
| specs.splice(idx, 1); | ||
|
cursor[bot] marked this conversation as resolved.
Outdated
|
||
|
|
||
| const existingLocals = new Set( | ||
| specs.filter(s => s.type === 'ImportSpecifier').map(s => s.local.name), | ||
| ); | ||
| for (const [imported, localName] of [...used.entries()].sort((a, b) => | ||
| a[0].localeCompare(b[0]), | ||
| )) { | ||
| if (!existingLocals.has(localName)) { | ||
| if (imported !== localName) { | ||
| specs.push( | ||
| j.importSpecifier(j.identifier(imported), j.identifier(localName)), | ||
| ); | ||
| } else { | ||
| specs.push(j.importSpecifier(j.identifier(imported))); | ||
| } | ||
| } | ||
| } | ||
|
cursor[bot] marked this conversation as resolved.
|
||
|
|
||
| dirty = true; | ||
| }); | ||
|
|
||
| return dirty; | ||
| } | ||
|
|
||
| // --- Main --- | ||
|
|
||
| module.exports = function transformer(fileInfo, api) { | ||
| const j = api.jscodeshift; | ||
| const root = j(fileInfo.source); | ||
| let dirty = false; | ||
|
|
||
| dirty = transformPaths(j, root) || dirty; | ||
| dirty = transformUseFetch(j, root) || dirty; | ||
| dirty = transformSchemaImports(j, root) || dirty; | ||
|
|
||
| return dirty ? root.toSource({ quote: 'single' }) : undefined; | ||
| }; | ||
|
|
||
| module.exports.parser = 'tsx'; | ||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.