fix(plugin-federation): add .js extension to the @apollo/subgraph subpath import - #1662
Merged
Conversation
…path import `esm/schema-builder.js` imported `@apollo/subgraph/dist/types` with no file extension. Node's ESM resolver does not guess extensions for package subpaths, and `@apollo/subgraph` publishes no `exports` map to remap it, so importing `@pothos/plugin-federation` from an ESM package threw ERR_MODULE_NOT_FOUND at startup on 4.4.3 through 4.5.0. CJS was unaffected because `require()` still resolves extensions. Up to 4.4.2 the ESM build appended extensions via scripts/esm-transformer.ts, which resolved bare subpaths through `require.resolve`. The TypeScript 7 migration replaced that step with extensions written in source and enforced by biome's useImportExtensions, but that rule only covers relative imports, so this specifier was left as it was. Add a test that checks package subpath imports in src against the target package's manifest: when the package has no `exports` map, the subpath has to name a file that exists. A sweep of every package's src found no other occurrence. Fixes #1661 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01L1YyNQHKx3pmp3zS9f9CMw
🦋 Changeset detectedLatest commit: d561d35 The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 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 |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
@pothos/core
@pothos/plugin-add-graphql
@pothos/plugin-complexity
@pothos/plugin-dataloader
@pothos/plugin-directives
@pothos/plugin-drizzle
@pothos/plugin-errors
@pothos/plugin-example
@pothos/plugin-federation
@pothos/plugin-grafast
@pothos/plugin-mocks
@pothos/plugin-prisma
@pothos/plugin-prisma-utils
@pothos/plugin-relay
@pothos/plugin-scope-auth
@pothos/plugin-simple-objects
@pothos/plugin-smart-subscriptions
@pothos/plugin-sub-graph
@pothos/plugin-tracing
@pothos/plugin-validation
@pothos/plugin-with-input
@pothos/plugin-zod
@pothos/tracing-newrelic
@pothos/tracing-opentelemetry
@pothos/tracing-sentry
@pothos/tracing-xray
commit: |
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
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.
Fixes #1661.
The bug
packages/plugin-federation/src/schema-builder.tsimports@apollo/subgraph/dist/typeswith no file extension. Node's ESM resolver does not guess extensions for package subpaths, and@apollo/subgraphpublishes noexportsmap that would remap it, so importing@pothos/plugin-federationfrom an ESM package throws at startup on 4.4.3 through 4.5.0:CJS is unaffected —
require()still resolves extensions, solib/works on both versions.Why it regressed
Up to 4.4.2 the ESM build appended extensions via
scripts/esm-transformer.ts, which resolved bare subpaths throughrequire.resolveand rewrote them. The TypeScript 7 migration (a32b270) replaced that step with extensions written in source and enforced by biome'suseImportExtensions. That rule only covers relative imports, so the relative specifiers in this file were updated and the bare subpath on line 2 was left as it was.The fix
One character of behavior:
@apollo/subgraph/dist/types→@apollo/subgraph/dist/types.js.dist/types.jsand its siblingdist/types.d.tsboth exist in the published package, so CJS, ESM, and type resolution all keep working.Other occurrences
I swept every
packages/*/srctree for non-relative subpath specifiers.@apollo/subgraph/dist/typeswas the only one. The other subpath imports in the repo are all inexamples/andwebsite/(next/*,drizzle-orm/*,@apollo/server/*,fumadocs-*), which are not published and whose targets all shipexportsmaps.Regression guard
Added
packages/plugin-federation/tests/esm-imports.test.ts. For each package subpath imported fromsrc, it reads the target package's manifest: if there is noexportsmap to remap the subpath, the specifier has to name a file that exists. This is the check biome's rule does not do.Verification
node --input-type=module -e "await import('./esm/index.js')"→ OK;node -e "require('./lib/index.js')"→ OK.pnpm turbo run build --filter='@pothos/plugin-federation...'succeeds; the builtesm/andlib/output both carrydist/types.js.pnpm vitest --runinpackages/plugin-federation: 3 files, 9 tests passed, no type errors.pnpm biome checkclean.One note on running the suite locally: the
superGraphtest intests/index.test.tsstarts local HTTP servers and fails with a405 Method Not Allowedif an HTTP proxy is set in the environment. Unsetting the proxy variables makes it pass. Unrelated to this change.🤖 Generated with Claude Code
https://claude.ai/code/session_01L1YyNQHKx3pmp3zS9f9CMw
Generated by Claude Code