fix(plugin): follow aliased/tsconfig-path imports into the MT worklet graph#190
Conversation
The MT worklet loader only re-emitted relative specifiers, so worklets reached via path aliases, tsconfig paths or bare packages were dropped from the MT module graph — their registerWorkletInternal never ran and the worklet threw 'cannot read property bind of undefined' at runtime. Switch worklet-loader-mt to an async loader and resolve each non-relative specifier via this.getResolve. Imports resolving to project/aliased source (outside node_modules) are now followed; imports into node_modules are followed only when allowlisted via the new includeWorkletPackages option (plus a node_modules exclude carve-out so the loader runs on them). Relative imports are unchanged. Unresolvable specifiers are skipped.
|
@KealanAU is attempting to deploy a commit to the huxpro's projects Team on Vercel. A member of the Team first needs to authorize it. |
… graph The MT worklet loader's import extractor matched `import … from …` text anywhere in a module, including JSDoc examples. Following aliased imports into vue-lynx's own source pulled in runtime/src/index.ts, whose docblock example `import App from './App.vue'` was re-emitted as a real import and failed to resolve, breaking the dev-smoke build. Strip line/block comments (preserving string/template literals) before extracting specifiers in extractImportSpecifiers and extractSharedImports.
|
Follow-up: CI Dev Smoke caught a build failure exposed by this change.
Parser-level fix rather than deleting the docblock — comment imports shouldn't be load-bearing |
…imports
Tokenize literals before scanning for import edges so import-like text
inside a string or template literal is never followed, and detect
`with { … }` attributes across line breaks so a reformatted
`runtime: 'shared'` import no longer leaks through as a plain local
import. Adds a parametrized parser test table plus emitted-output and
resolver-rejection regression guards.
Huxpro
left a comment
There was a problem hiding this comment.
Blocking from my side: includeWorkletPackages now documents/tests RegExp support, but the matching model is still inconsistent across the code path this PR is fixing.
At one decision point the regex is matched against the original import specifier/package name, but when the traversal logic decides whether to keep following into the resolved node_modules target, the effective input is the resolved absolute path. That means a documented pattern like /^@my-org\// will match @my-org/foo but can never match something like /project/node_modules/@my-org/foo/dist/index.js, so RegExp users can still silently miss worklets here.
I think this needs one stable matching input at both checkpoints, ideally the original package specifier/package root rather than a filesystem path.
…ame at both checkpoints The node_modules loader-exclude matched the allowlist against the resolved absolute path while the import-follow checkpoint matched the original specifier. A RegExp like /^@my-org\// matched @my-org/foo but never .../node_modules/@my-org/foo/dist/index.js, silently dropping the package. Derive the package name from the resolved path (packageNameFromNodeModulesPath) and route both checkpoints through isWorkletPackage, so the allowlist always matches a package specifier, never a filesystem path.
…gh the loader End-to-end loader tests only used string allowlists, which pass even if RegExp handling regresses. Add a runLoaderMT test that follows a bare npm import via /^@org\// (resolver returns an absolute node_modules path) and confirms a non-matching RegExp still drops it.
Previously checkpoint A matched the full import specifier while checkpoint B matched the package root derived from the resolved path. Equivalent for package-name patterns but divergent for subpath-sensitive ones. Add packageRootFromSpecifier and reduce both inputs to the package root before matching, so a specifier and a resolved path always compare as the same thing. The allowlist matcher is now a plain exact/RegExp compare.
|
Addressed — both checkpoints now match one stable input. I added Concretely, Pinned by tests:
Commits: 980f3f1 (route both checkpoints through |
Huxpro
left a comment
There was a problem hiding this comment.
Thanks for the thorough follow-through here. The matching model is now consistent across both checkpoints — reducing specifiers and resolved paths to a package root and running both through a single isWorkletPackage matcher fully addresses my earlier concern, and the path-vs-specifier RegExp parity test plus the end-to-end subpath case make it hard to regress. The comment/literal masking is a clean parser-level fix for the JSDoc-import build failure rather than a one-off patch. Behavior stays backward-compatible (only additive graph edges; unresolvable specifiers are skipped), CI is green, and the 46 tests cover the fragile surfaces well. LGTM.
(The red Vercel status is just the external-contributor deploy-authorization gate, not a code failure.)
Generated by Claude Code
Huxpro
left a comment
There was a problem hiding this comment.
Second-pass review complete. Fixed the stateful RegExp allowlist edge case by testing against a cloned expression, added regression coverage for both global and sticky flags (including preserving caller lastIndex), and reran the focused suite (48/48), lint, and the full monorepo build locally. All six repository CI checks are green on 3c01812; the remaining Vercel status is the known fork authorization failure rather than a code failure.
…mports fix(plugin): follow aliased/tsconfig-path imports into the MT worklet graph
Fixes #191.
Summary
MT worklets imported through aliases, tsconfig paths, or package specifiers were dropped from the main-thread graph, so their registrations never ran.
worklet-loader-mtasync and resolves non-relative imports withthis.getResolve(...).node_modulesand re-emits the original specifier for bundler resolution.includeWorkletPackages(string orRegExp) for packages that intentionally ship MT worklets.Allowlist matching (addresses review)
Both decision points now reduce their input to a package root before matching, so a specifier and a resolved path always compare as the same thing.
packageRootFromSpecifierreduces an import specifier (@org/foo/dist/x→@org/foo).packageNameFromNodeModulesPathreduces a resolved path (…/node_modules/@org/foo/dist/index.js→@org/foo), using the lastnode_modulessegment so nested deps and pnpm layouts resolve.isWorkletPackagematcher, so/^@org\//matches at the follow checkpoint AND the loader carve-out — no more silent misses forRegExpusers.Safety
Verification
worklet-loader-imports.test.ts: 46 tests, including a path-vs-specifier RegExp parity test and end-to-end RegExp/subpath cases through the real loader.