Fix v0.16 codemod: add shadow checks to transformUseFetch#3836
Merged
Fix v0.16 codemod: add shadow checks to transformUseFetch#3836
Conversation
jscodeshift transform served from dataclient.io/codemods/v0.16.js handling path-to-regexp v8 syntax, useFetch() .resolved, and schema namespace to direct imports. Linked from the blog post migration guide. Made-with: Cursor
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #3836 +/- ##
=======================================
Coverage 98.07% 98.07%
=======================================
Files 151 151
Lines 2855 2855
Branches 561 561
=======================================
Hits 2800 2800
Misses 11 11
Partials 44 44 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
…n same declaration When a name like Entity is both directly imported and used via schema.Entity in the same import declaration, the codemod had two bugs: 1. scopeBindings included specifiers from the same import declaration (other than the schema specifier itself), causing resolveLocal to incorrectly detect a conflict and rename schema.Entity to SchemaEntity. Fixed by skipping the entire current import declaration when building scopeBindings, since those names come from the same package and won't conflict. 2. The existing-import dedup check compared imported names (e.g. 'Entity') rather than local names. When an alias was needed (Entity as SchemaEntity), finding 'Entity' already present caused the aliased specifier to be skipped, leaving SchemaEntity undefined. Fixed by checking existingLocals (s.local.name) against localName instead. Co-authored-by: Nathaniel Tucker <me@ntucker.me>
transformUseFetch previously collected variable names into a file-global set, then rewrote every matching identifier in any if/ternary/&& across the entire file. If two components in the same file both declared 'const data = ...' — one from useFetch() and one from an unrelated source — the non-useFetch truthiness check would be incorrectly rewritten to '!data.resolved'. Fix by walking up to the enclosing function for each useFetch() declarator and only rewriting truthiness checks within that scope. Co-authored-by: Nathaniel Tucker <me@ntucker.me>
The rewrite function transformed `if (promise)` into `if (!promise.resolved)`, but useFetch returns undefined when called with null args (conditional fetching). Accessing .resolved on undefined throws a TypeError at runtime. Use optional chaining (promise?.resolved) to guard against undefined. Co-authored-by: Nathaniel Tucker <me@ntucker.me>
cursor
bot
changed the title
The LogicalExpression handler only called rewrite(p.node.left), so a useFetch variable on the right side of && or || (e.g. 'condition && promise') was never transformed. In v0.16, promise is always a truthy UsablePromise object when args are non-null, so untransformed right-side checks silently became no-ops. Now both left and right sides are rewritten, and || expressions are handled alongside &&. Co-authored-by: Nathaniel Tucker <me@ntucker.me>
…exist The transformSchemaImports function was unconditionally removing the schema import specifier after rewriting schema.X member expressions, but standalone references to 'schema' as a bare identifier (destructuring, function arguments, typeof) were left unrewritten, causing ReferenceError at runtime. Now checks for remaining bare references before removing the import. If any exist, the schema specifier is kept while still adding the individual named imports for the rewritten member expressions. Co-authored-by: Nathaniel Tucker <me@ntucker.me>
ntucker
changed the title
Object and Array are type-only exports from @data-client/endpoint
(export type { Array, Object }), not value exports. The codemod was
transforming schema.Object/schema.Array into direct imports like
'import { Object as SchemaObject }', which would fail in value
positions (e.g. new SchemaObject(...)). These members must remain
accessed via the schema namespace.
Co-authored-by: Nathaniel Tucker <me@ntucker.me>
…ata-client transformUseFetch previously matched any call named useFetch without verifying it was imported from a @data-client package. Since useFetch is a common hook name exported by other libraries (e.g. use-http), running this codemod on codebases using a different useFetch would silently corrupt truthiness checks. Added an import-source guard that checks for a useFetch import from any @data-client/* package before applying the transformation, consistent with how transformSchemaImports gates on DATA_CLIENT_PACKAGES. Co-authored-by: Nathaniel Tucker <me@ntucker.me>
cursor
bot
changed the title
…l binding
transformSchemaImports was replacing ALL schema.X member expressions
file-wide without checking whether 'schema' at each usage site actually
referred to the import or to a local variable/parameter that shadows it.
For example, a function parameter named 'schema' (e.g.,
function validate(schema) { return schema.isValid; }) in the same file
as import { schema } from '@data-client/endpoint' would have
schema.isValid incorrectly replaced with a bare isValid identifier.
Add isShadowed() helper that walks up the AST from each schema.X node
to check for shadowing bindings in enclosing scopes (function params,
catch clauses, variable declarations, for-loop bindings). Skip the
replacement when the identifier is shadowed.
Co-authored-by: Nathaniel Tucker <me@ntucker.me>
cursor
bot
changed the title
…ching The rewrite function transformed: - `promise` → `!promise?.resolved` - `!promise` → `promise?.resolved` This inverts boolean semantics when useFetch returns undefined (null args for conditional fetching). `undefined?.resolved` evaluates to `undefined` via optional chaining, so `!undefined` is `true` — but the original `if (promise)` with `undefined` was `false`. Fix by using strict equality against `false`: - `promise` → `promise?.resolved === false` - `!promise` → `promise?.resolved !== false` This preserves the original semantics for all three states: in-flight promise (resolved=false), resolved promise (resolved=true), and disabled fetch (undefined). Co-authored-by: Nathaniel Tucker <me@ntucker.me>
…ndings scopeBindings only collected local names from import declarations, missing top-level variable, function, and class declarations. This caused resolveLocal to miss naming conflicts (e.g. const Union = someValue), producing invalid code with duplicate bindings. Now collects bindings from: - Top-level VariableDeclarations (including destructured patterns) - Top-level FunctionDeclarations - Top-level ClassDeclarations - Exported variants of all the above Co-authored-by: Nathaniel Tucker <me@ntucker.me>
…n isShadowed The isShadowed function only checked VariableDeclaration statements when scanning BlockStatement/Program nodes. This missed FunctionDeclaration and ClassDeclaration names that also create bindings in block scope. When a local function or class with the same name as the import alias (e.g. 'schema') existed in an enclosing block, isShadowed incorrectly returned false, causing the codemod to transform references to those local bindings. Co-authored-by: Nathaniel Tucker <me@ntucker.me>
transformUseFetch searched the entire enclosing function for truthiness checks on the useFetch variable name, but never checked whether that name was shadowed by a parameter or local variable in a nested function or callback. This caused incorrect rewrites like adding .resolved to a callback parameter that happened to share the same name. Fix: reuse the existing isShadowed() helper (already used by transformSchemaImports) with an optional stopNode parameter to limit the shadow walk to scopes between the found expression and the function where useFetch was declared. Co-authored-by: Nathaniel Tucker <me@ntucker.me>
cursor
bot
changed the title
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
transformPaths was transforming every StringLiteral with a 'path' property
key in the entire file, regardless of whether the file uses @data-client.
This could silently corrupt path strings from unrelated libraries (React
Router, Express, etc.) like /:param? → {/:param}.
Add the same DATA_CLIENT_PACKAGES import guard that transformUseFetch
already uses, so transformPaths only runs on files that actually import
from @data-client/endpoint or @data-client/rest.
Co-authored-by: Nathaniel Tucker <me@ntucker.me>
…licts resolveLocal prefixed a name with 'Schema' when it collided with a JS global or scope binding, but never verified the resulting identifier was itself free of collisions. If the user had an existing binding named e.g. SchemaUnion, the codemod would silently produce a conflicting identifier. Now the function loops, prepending '_' to the candidate until it finds an identifier that doesn't collide with JS_GLOBALS or scopeBindings. Co-authored-by: Nathaniel Tucker <me@ntucker.me>
pull bot
pushed a commit
to erickirt/data-client
that referenced
this pull request
Mar 30, 2026
) * docs: Add v0.16 migration codemod jscodeshift transform served from dataclient.io/codemods/v0.16.js handling path-to-regexp v8 syntax, useFetch() .resolved, and schema namespace to direct imports. Linked from the blog post migration guide. Made-with: Cursor * fix: bugbot * Fix schema codemod: aliased import skipped when name already exists in same declaration When a name like Entity is both directly imported and used via schema.Entity in the same import declaration, the codemod had two bugs: 1. scopeBindings included specifiers from the same import declaration (other than the schema specifier itself), causing resolveLocal to incorrectly detect a conflict and rename schema.Entity to SchemaEntity. Fixed by skipping the entire current import declaration when building scopeBindings, since those names come from the same package and won't conflict. 2. The existing-import dedup check compared imported names (e.g. 'Entity') rather than local names. When an alias was needed (Entity as SchemaEntity), finding 'Entity' already present caused the aliased specifier to be skipped, leaving SchemaEntity undefined. Fixed by checking existingLocals (s.local.name) against localName instead. Co-authored-by: Nathaniel Tucker <me@ntucker.me> * fix: scope useFetch truthiness rewrites to enclosing function transformUseFetch previously collected variable names into a file-global set, then rewrote every matching identifier in any if/ternary/&& across the entire file. If two components in the same file both declared 'const data = ...' — one from useFetch() and one from an unrelated source — the non-useFetch truthiness check would be incorrectly rewritten to '!data.resolved'. Fix by walking up to the enclosing function for each useFetch() declarator and only rewriting truthiness checks within that scope. Co-authored-by: Nathaniel Tucker <me@ntucker.me> * fix(codemod): use optional chaining in useFetch truthiness rewrite The rewrite function transformed `if (promise)` into `if (!promise.resolved)`, but useFetch returns undefined when called with null args (conditional fetching). Accessing .resolved on undefined throws a TypeError at runtime. Use optional chaining (promise?.resolved) to guard against undefined. Co-authored-by: Nathaniel Tucker <me@ntucker.me> * fix(codemod): rewrite useFetch vars on both sides of LogicalExpression The LogicalExpression handler only called rewrite(p.node.left), so a useFetch variable on the right side of && or || (e.g. 'condition && promise') was never transformed. In v0.16, promise is always a truthy UsablePromise object when args are non-null, so untransformed right-side checks silently became no-ops. Now both left and right sides are rewritten, and || expressions are handled alongside &&. Co-authored-by: Nathaniel Tucker <me@ntucker.me> * fix(codemod): preserve schema import when bare identifier references exist The transformSchemaImports function was unconditionally removing the schema import specifier after rewriting schema.X member expressions, but standalone references to 'schema' as a bare identifier (destructuring, function arguments, typeof) were left unrewritten, causing ReferenceError at runtime. Now checks for remaining bare references before removing the import. If any exist, the schema specifier is kept while still adding the individual named imports for the rewritten member expressions. Co-authored-by: Nathaniel Tucker <me@ntucker.me> * fix: bugbot * fix(codemod): skip schema.Object and schema.Array in v0.16 transform Object and Array are type-only exports from @data-client/endpoint (export type { Array, Object }), not value exports. The codemod was transforming schema.Object/schema.Array into direct imports like 'import { Object as SchemaObject }', which would fail in value positions (e.g. new SchemaObject(...)). These members must remain accessed via the schema namespace. Co-authored-by: Nathaniel Tucker <me@ntucker.me> * Fix transformUseFetch to only apply when useFetch is imported from @data-client transformUseFetch previously matched any call named useFetch without verifying it was imported from a @data-client package. Since useFetch is a common hook name exported by other libraries (e.g. use-http), running this codemod on codebases using a different useFetch would silently corrupt truthiness checks. Added an import-source guard that checks for a useFetch import from any @data-client/* package before applying the transformation, consistent with how transformSchemaImports gates on DATA_CLIENT_PACKAGES. Co-authored-by: Nathaniel Tucker <me@ntucker.me> * fix(codemod): skip schema.X rewrites where schema is shadowed by local binding transformSchemaImports was replacing ALL schema.X member expressions file-wide without checking whether 'schema' at each usage site actually referred to the import or to a local variable/parameter that shadows it. For example, a function parameter named 'schema' (e.g., function validate(schema) { return schema.isValid; }) in the same file as import { schema } from '@data-client/endpoint' would have schema.isValid incorrectly replaced with a bare isValid identifier. Add isShadowed() helper that walks up the AST from each schema.X node to check for shadowing bindings in enclosing scopes (function params, catch clauses, variable declarations, for-loop bindings). Skip the replacement when the identifier is shadowed. Co-authored-by: Nathaniel Tucker <me@ntucker.me> * fix(codemod): preserve boolean semantics for useFetch conditional fetching The rewrite function transformed: - `promise` → `!promise?.resolved` - `!promise` → `promise?.resolved` This inverts boolean semantics when useFetch returns undefined (null args for conditional fetching). `undefined?.resolved` evaluates to `undefined` via optional chaining, so `!undefined` is `true` — but the original `if (promise)` with `undefined` was `false`. Fix by using strict equality against `false`: - `promise` → `promise?.resolved === false` - `!promise` → `promise?.resolved !== false` This preserves the original semantics for all three states: in-flight promise (resolved=false), resolved promise (resolved=true), and disabled fetch (undefined). Co-authored-by: Nathaniel Tucker <me@ntucker.me> * fix(codemod): detect top-level var/function/class bindings in scopeBindings scopeBindings only collected local names from import declarations, missing top-level variable, function, and class declarations. This caused resolveLocal to miss naming conflicts (e.g. const Union = someValue), producing invalid code with duplicate bindings. Now collects bindings from: - Top-level VariableDeclarations (including destructured patterns) - Top-level FunctionDeclarations - Top-level ClassDeclarations - Exported variants of all the above Co-authored-by: Nathaniel Tucker <me@ntucker.me> * fix(codemod): detect FunctionDeclaration/ClassDeclaration shadowing in isShadowed The isShadowed function only checked VariableDeclaration statements when scanning BlockStatement/Program nodes. This missed FunctionDeclaration and ClassDeclaration names that also create bindings in block scope. When a local function or class with the same name as the import alias (e.g. 'schema') existed in an enclosing block, isShadowed incorrectly returned false, causing the codemod to transform references to those local bindings. Co-authored-by: Nathaniel Tucker <me@ntucker.me> * Fix transformUseFetch rewriting shadowed variables in nested callbacks transformUseFetch searched the entire enclosing function for truthiness checks on the useFetch variable name, but never checked whether that name was shadowed by a parameter or local variable in a nested function or callback. This caused incorrect rewrites like adding .resolved to a callback parameter that happened to share the same name. Fix: reuse the existing isShadowed() helper (already used by transformSchemaImports) with an optional stopNode parameter to limit the shadow walk to scopes between the found expression and the function where useFetch was declared. Co-authored-by: Nathaniel Tucker <me@ntucker.me> * Guard transformPaths with @data-client import check transformPaths was transforming every StringLiteral with a 'path' property key in the entire file, regardless of whether the file uses @data-client. This could silently corrupt path strings from unrelated libraries (React Router, Express, etc.) like /:param? → {/:param}. Add the same DATA_CLIENT_PACKAGES import guard that transformUseFetch already uses, so transformPaths only runs on files that actually import from @data-client/endpoint or @data-client/rest. Co-authored-by: Nathaniel Tucker <me@ntucker.me> * Fix resolveLocal collision: verify 'Schema'+name is also free of conflicts resolveLocal prefixed a name with 'Schema' when it collided with a JS global or scope binding, but never verified the resulting identifier was itself free of collisions. If the user had an existing binding named e.g. SchemaUnion, the codemod would silently produce a conflicting identifier. Now the function loops, prepending '_' to the candidate until it finds an identifier that doesn't collide with JS_GLOBALS or scopeBindings. Co-authored-by: Nathaniel Tucker <me@ntucker.me> --------- Co-authored-by: Cursor Agent <cursoragent@cursor.com>
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Motivation
Users upgrading to v0.16 face three breaking changes that require manual code updates across their codebase. An automated codemod makes this migration significantly easier and less error-prone.
Solution
Add a jscodeshift codemod served as a static asset at
dataclient.io/codemods/v0.16.js. Users can run it directly via remote URL:The codemod handles all three breaking changes:
/:param?→{/:param},/:name+→/*name,/:name*→{/*name}, inline regex removal, escape cleanupif (promise)→if (!promise.resolved)in if/ternary/&& expressionsnew schema.Union(...)→new Union(...)with import rewriting (including TS type positions)The blog post migration guide section now links to the codemod with a copy-pasteable one-liner.
Open questions
N/A
Made with Cursor
Note
Medium Risk
The new codemod performs non-trivial AST rewrites across user code, so incorrect matching or shadowing edge cases could cause unintended transformations; site/docs changes themselves are low risk.
Overview
Adds a hosted
jscodeshiftcodemod atwebsite/static/codemods/v0.16.jsto automate the v0.16 migration: rewritesRestEndpoint/resourcepathstrings for path-to-regexp v8, convertsuseFetch()truthiness checks to.resolvedcomparisons (with identifier shadowing safeguards), and replacesschema.Xmember access with direct schema imports (including TS type positions).Updates the v0.16 release announcement migration guide to link to the codemod and provide a copy-paste
npx jscodeshift -t https://dataclient.io/codemods/v0.16.js ...command.Written by Cursor Bugbot for commit b73054e. This will update automatically on new commits. Configure here.