Uh oh!
There was an error while loading. Please reload this page.
Add subpath export resolution for package IDs - #901
Merged
TooTallNate merged 8 commits intoFeb 5, 2026
Merged
Conversation
🦋 Changeset detectedLatest commit: b81ae6c The changes in this PR will be included in the next version bump. This PR includes changesets to release 17 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Contributor
Contributor
🧪 E2E Test Results❌ Some tests failed Summary
❌ Failed Tests🌍 Community Worlds (169 failed)mongodb (42 failed):
redis (42 failed):
starter (43 failed):
turso (42 failed):
Details by Category✅ ▲ Vercel Production
✅ 💻 Local Development
✅ 📦 Local Production
✅ 🐘 Local Postgres
✅ 🪟 Windows
❌ 🌍 Community Worlds
✅ 📋 Other
|
MemberAuthor
This stack of pull requests is managed by Graphite. Learn more about stacking. |
This was referenced Jan 30, 2026
TooTallNateforce-pushed
the
01-30-fix_module_specifier_cache_bug_and_add_subpath_export_resolution_for_package_ids
branch
from
January 30, 2026 19:57
6cce073 to
ca8bd01CompareContributor
📊 Benchmark Results
workflow with no steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Express | Next.js (Turbopack) workflow with 1 step💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Express | Next.js (Turbopack) workflow with 10 sequential steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Next.js (Turbopack) | Nitro | Express workflow with 25 sequential steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Nitro | Next.js (Turbopack) workflow with 50 sequential steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Next.js (Turbopack) | Express Promise.all with 10 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Next.js (Turbopack) | Express Promise.all with 25 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Nitro | Next.js (Turbopack) Promise.all with 50 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Express | Next.js (Turbopack) Promise.race with 10 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Next.js (Turbopack) | Express Promise.race with 25 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Nitro | Next.js (Turbopack) Promise.race with 50 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Express | Next.js (Turbopack) Stream Benchmarks(includes TTFB metrics)workflow with stream💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Next.js (Turbopack) | Express SummaryFastest Framework by WorldWinner determined by most benchmark wins
Fastest World by FrameworkWinner determined by most benchmark wins
Column Definitions
Worlds:
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes a caching bug in the module specifier resolution system and adds support for package subpath exports in workflow IDs. The changes improve the reliability of cross-bundle references and enable proper handling of packages with multiple entry points.
Changes:
- Fixed module specifier cache back-filling to prevent incorrect IDs across multiple lookups
- Added subpath export resolution to support packages like
workflow/internal/builtins@4.0.0 - Enhanced workspace package detection to filter out sibling apps in monorepos based on project dependencies
- Updated all builders to properly merge manifests from both workflow and step bundles
- Added Windows path normalization support in the Rust transform plugin
- Passed absolute file paths to SWC transform for accurate module specifier resolution
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/builders/src/module-specifier.ts | Core changes: cache back-filling, subpath export resolution, workspace package detection improvements |
| packages/builders/src/apply-swc-transform.ts | Added absolutePath parameter for accurate module specifier resolution |
| packages/builders/src/swc-esbuild-plugin.ts | Passes absolute path to transform function |
| packages/builders/src/base-builder.ts | Updated createWorkflowsBundle return type to include manifest |
| packages/sveltekit/src/builder.ts | Updated to merge manifests from both bundles |
| packages/nitro/src/builders.ts | Updated to merge manifests from both bundles |
| packages/next/src/builder.ts | Updated to merge manifests and handle optional context properties |
| packages/nest/src/builder.ts | Updated to merge manifests from both bundles |
| packages/builders/src/vercel-build-output-api.ts | Updated to merge manifests from both bundles |
| packages/builders/src/standalone.ts | Added mergeManifests helper and updated bundle methods |
| packages/swc-plugin-workflow/transform/src/naming.rs | Added Windows path normalization with tests |
| packages/swc-plugin-workflow/transform/src/lib.rs | Enhanced builtin function comment documentation |
| packages/swc-plugin-workflow/spec.md | Updated documentation for subpath exports |
| .changeset/afraid-candies-find.md | Added changeset entry |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
37
to
327
| @@ -69,9 +176,56 @@ function isInNodeModules(filePath: string): boolean { | |||
| } | |||
| /** | |||
| * Check if a file path is inside a workspace package. | |||
| * Cache for project dependencies to avoid repeated filesystem reads. | |||
| * Maps project root to set of dependency package names. | |||
| */ | |||
| const projectDepsCache = new Map<string, Set<string>>(); | |||
| /** | |||
| * Get all dependencies (including devDependencies) for a project. | |||
| */ | |||
| function getProjectDependencies(projectRoot: string): Set<string> { | |||
| const cached = projectDepsCache.get(projectRoot); | |||
| if (cached) { | |||
| return cached; | |||
| } | |||
| const deps = new Set<string>(); | |||
| const pkgPath = join(projectRoot, 'package.json'); | |||
| if (existsSync(pkgPath)) { | |||
| try { | |||
| const content = readFileSync(pkgPath, 'utf-8'); | |||
| const parsed = JSON.parse(content); | |||
| // Collect all dependency types | |||
| for (const depType of [ | |||
| 'dependencies', | |||
| 'devDependencies', | |||
| 'peerDependencies', | |||
| 'optionalDependencies', | |||
| ]) { | |||
| const depObj = parsed[depType]; | |||
| if (depObj && typeof depObj === 'object') { | |||
| for (const name of Object.keys(depObj)) { | |||
| deps.add(name); | |||
| } | |||
| } | |||
| } | |||
| } catch { | |||
| // Invalid JSON or file not readable | |||
| } | |||
| } | |||
| projectDepsCache.set(projectRoot, deps); | |||
| return deps; | |||
| } | |||
| /** | |||
| * Check if a file path is inside a workspace package that is a dependency of the project. | |||
| * This is a heuristic - we check if the file is in a directory with a package.json | |||
| * that has a "name" field, but is NOT in node_modules. | |||
| * that has a "name" field, is NOT in node_modules, and is listed as a dependency | |||
| * of the project. | |||
| */ | |||
| function isWorkspacePackage(filePath: string, projectRoot: string): boolean { | |||
| if (isInNodeModules(filePath)) { | |||
| @@ -97,8 +251,13 @@ function isWorkspacePackage(filePath: string, projectRoot: string): boolean { | |||
| if (resolve(pkgPath) === rootPkgPath) { | |||
| return false; | |||
| } | |||
| // Found a package.json that's not the root - it's a workspace package | |||
| return true; | |||
| // Found a package.json that's not the root. | |||
| // Only treat it as a workspace package if it's actually a dependency | |||
| // of the current project. This prevents sibling apps in a monorepo | |||
| // from being incorrectly treated as importable packages. | |||
| const projectDeps = getProjectDependencies(projectRoot); | |||
| return projectDeps.has(pkg.name); | |||
| } | |||
| dir = dirname(dir); | |||
| } | |||
| @@ -114,11 +273,16 @@ function isWorkspacePackage(filePath: string, projectRoot: string): boolean { | |||
| * @returns The module specifier result | |||
| * | |||
| * @example | |||
| * // File in node_modules | |||
| * // File in node_modules (root export) | |||
| * resolveModuleSpecifier('/project/node_modules/point/dist/index.js', '/project') | |||
| * // => { moduleSpecifier: 'point@1.0.0' } | |||
| * | |||
| * @example | |||
| * // File in node_modules (subpath export) | |||
| * resolveModuleSpecifier('/project/node_modules/workflow/dist/internal/builtins.js', '/project') | |||
| * // => { moduleSpecifier: 'workflow/internal/builtins@4.0.0' } | |||
| * | |||
| * @example | |||
| * // File in workspace package | |||
| * resolveModuleSpecifier('/project/packages/shared/src/utils.ts', '/project') | |||
| * // => { moduleSpecifier: '@myorg/shared@0.0.0' } | |||
| @@ -149,9 +313,16 @@ export function resolveModuleSpecifier( | |||
| return { moduleSpecifier: undefined }; | |||
| } | |||
| // Return the module specifier as "name@version" | |||
| // Resolve the export subpath (e.g., "/internal/builtins" for "workflow/internal/builtins") | |||
| const subpath = resolveExportSubpath(filePath, pkg); | |||
| // Return the module specifier as "name/subpath@version" or "name@version" | |||
| const specifier = subpath | |||
| ? `${pkg.name}${subpath}@${pkg.version}` | |||
| : `${pkg.name}@${pkg.version}`; | |||
| return { | |||
| moduleSpecifier: `${pkg.name}@${pkg.version}`, | |||
| moduleSpecifier: specifier, | |||
| }; | |||
| } | |||
There was a problem hiding this comment.
The new module specifier resolution logic, including the caching improvements, workspace package detection based on project dependencies, and subpath export resolution, lacks test coverage. Consider adding comprehensive tests to verify:
- The cache back-filling logic works correctly for nested directories
- Subpath export resolution correctly handles various export configurations (conditional exports, nested subpaths, etc.)
- The workspace package detection correctly filters out sibling apps in a monorepo
- The cache is properly invalidated when needed
Tests would help prevent regressions and document the expected behavior of these critical features.
Comment on lines
+61
to
+66
| // Merge manifests from both bundles | ||
| const manifest = { | ||
| steps: { ...stepsManifest.steps, ...workflowsManifest.steps }, | ||
| workflows: { ...stepsManifest.workflows, ...workflowsManifest.workflows }, | ||
| classes: { ...stepsManifest.classes, ...workflowsManifest.classes }, | ||
| }; |
There was a problem hiding this comment.
The manifest merging order could potentially cause data loss if there are conflicting keys between the stepsManifest and workflowsManifest. The current implementation uses object spread syntax where workflowsManifest properties overwrite stepsManifest properties with the same key.
While this might be intentional, consider:
- Adding validation to detect and warn about conflicting IDs
- Documenting which manifest takes precedence and why
- Ensuring that the merge order is consistent across all builders (currently stepsManifest comes first in all implementations)
If steps and workflows are guaranteed to have non-overlapping IDs by design, this concern can be disregarded, but it would be helpful to document this assumption.
Comment on lines
+100
to
+130
| function resolveExportSubpath(filePath: string, pkg: PackageInfo): string { | ||
| if (!pkg.exports || typeof pkg.exports !== 'object') { | ||
| return ''; | ||
| } | ||
| // Get the relative path from package root to the file | ||
| const normalizedFilePath = filePath.replace(/\\/g, '/'); | ||
| const normalizedPkgDir = pkg.dir.replace(/\\/g, '/'); | ||
| const relativePath = normalizedFilePath.startsWith(normalizedPkgDir + '/') | ||
| ? './' + normalizedFilePath.substring(normalizedPkgDir.length + 1) | ||
| : null; | ||
| if (!relativePath) { | ||
| return ''; | ||
| } | ||
| // Search through exports to find a matching subpath | ||
| for (const [subpath, target] of Object.entries(pkg.exports)) { | ||
| const resolvedTarget = resolveExportTarget(target); | ||
| if ( | ||
| resolvedTarget && | ||
| normalizeExportPath(resolvedTarget) === relativePath | ||
| ) { | ||
| // Found a match - return the subpath without the leading "." | ||
| // e.g., "./internal/builtins" -> "/internal/builtins" | ||
| return subpath === '.' ? '' : subpath.substring(1); | ||
| } | ||
| } | ||
| return ''; | ||
| } |
There was a problem hiding this comment.
The resolveExportSubpath function only handles exact export path matches and doesn't support wildcard patterns (e.g., "./internal/": "./dist/internal/.js"). This could lead to incorrect or missing subpath resolution for packages that use wildcard exports. Consider adding support for wildcard patterns by:
- Checking if the subpath contains an asterisk
- Converting it to a pattern match against the relative file path
- Extracting the matched portion to construct the correct subpath
If wildcard exports are not expected to be used in this codebase, this concern can be disregarded.
Comment on lines
+105
to
+114
| // Get the relative path from package root to the file | ||
| const normalizedFilePath = filePath.replace(/\\/g, '/'); | ||
| const normalizedPkgDir = pkg.dir.replace(/\\/g, '/'); | ||
| const relativePath = normalizedFilePath.startsWith(normalizedPkgDir + '/') | ||
| ? './' + normalizedFilePath.substring(normalizedPkgDir.length + 1) | ||
| : null; | ||
| if (!relativePath) { | ||
| return ''; | ||
| } |
There was a problem hiding this comment.
The resolveExportSubpath function checks if the normalized file path starts with the package directory plus a forward slash (normalizedPkgDir + '/'). This check will fail for files that are exactly at the package root (e.g., when filePath equals pkg.dir), causing the function to return an empty string even though it should potentially match the root export (".").
While files at the exact package root are uncommon, consider handling this edge case explicitly:
- Check if normalizedFilePath equals normalizedPkgDir and return the root export logic
- Or adjust the substring logic to handle this case
This may not be a practical issue if package entry points are always in subdirectories.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
TooTallNate
commented
Feb 4, 2026
MemberAuthor
Addressed the following review comments in commit fce2e7d:
|
… exports support - Remove redundant directory walk in isWorkspacePackage by using pkg.dir from findPackageJson - Add support for array exports in resolveExportTarget (fallback chains per Node.js spec)
VaguelySerious
approved these changes
Feb 5, 2026
VaguelySerious
left a comment
Member
There was a problem hiding this comment.
Enhanced manifest merging to properly combine results from both workflow and step bundles
Out of curiosity, what does this fix? "Enhance" sounds nice but doesn't tell me anything
TooTallNate
commented
Feb 5, 2026
MemberAuthor
@VaguelySerious The step and workflow bundles generate their own manifests, which may have some values that do not overlap. For example, if I use a class only in step functions, but never reference it in the workflow bundle, then it would be missing from the workflow manifest. The fix was to ensure that all bundles are merged in the final form. |
Merged
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 freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Fixed a bug in module specifier resolution and added support for package subpath exports in workflow IDs.
What changed?
workflow/internal/builtins@4.0.0)How to test?
workflow/internal/builtins@4.0.0)Why make this change?
This change addresses an issue where the module specifier cache could return incorrect results, leading to inconsistent workflow IDs. It also adds support for packages with multiple entry points through subpath exports, ensuring that steps with the same name in different subpaths don't collide. This improves the reliability of cross-bundle references and makes the system more robust when working with complex package structures.