Fix imports for moduleResolution: nodenext TS users (v4)
#6731
Conversation
|
✅ Deploy Preview for apollo-server-docs ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit 8e48da7:
|
|
I'm able to reliably reproduce and then resolve the issue via this codesandbox: https://codesandbox.io/s/hungry-bird-mzs69d There seems to be some package caching, so it needs a thorough clean to reproduce.
|
|
can you smoke-test it? (and ideally eslint but ISTR that wasn't possible) |
|
@glasser I tried to reproduce this locally by cherry picking the new smoke test fcb0295 commit onto Needs more investigation, and possibly a more complex configuration than our current smoke tests use. i.e. I think I need a |
trevor-scheer
force-pushed
the
trevor/fix-imports-v4
branch
4 times, most recently
from
c69326b to
e56d168
Compare
trevor-scheer
force-pushed
the
trevor/fix-imports-v4
branch
from
e56d168 to
1a414a4
Compare
|
My previous comment was correct - the package.json |
There was a problem hiding this comment.
It's a bummer that AFAIK we can't make eslint check this (since that's a lot faster than smoke-test). import-js/eslint-plugin-import#2270 ...
smoke-test/nodenext/package.json
Outdated
| "type": "module", | ||
| "module": "./dist/smoke-test.js", | ||
| "dependencies": { | ||
| "@apollo/server": "../node_modules/@apollo/server", |
There was a problem hiding this comment.
The rest of the smoketests install a built tarball — is that OK? or is this reading from inside smoke-test anyway i guess... huh.
There was a problem hiding this comment.
Simplified the reproduction a bit, and you're right that it's just looking up.
cee95bb
trevor-scheer
changed the title
nodenext TS users (v4)moduleResolution: nodenext TS users (v4)
This PR was opened by the [Changesets release](https://github.com/changesets/action) GitHub action. When you're ready to do a release, you can merge this and the packages will be published to npm automatically. If you're not ready to do a release yet, that's fine, whenever you add more changesets to version-4, this PR will be updated.⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ `version-4` is currently in **pre mode** so this branch has prereleases rather than normal releases. If you want to exit prereleases, run `changeset pre exit` on `version-4`.⚠️ ⚠️ ⚠️ ⚠️ ⚠️ ⚠️ # Releases ## @apollo/[email protected] ### Patch Changes - Updated dependencies \[[`3320fee92`](3320fee), [`9fc23f799`](9fc23f7), [`2cab8f785`](2cab8f7)]: - @apollo/[email protected] ## @apollo/[email protected] ### Patch Changes - [#6841](#6841) [`3320fee92`](3320fee) Thanks [@glasser](https://github.com/glasser)! - Upgrade @apollo/server-gateway-interface to have laxer definition of overallCachePolicy. - [#6731](#6731) [`9fc23f799`](9fc23f7) Thanks [@trevor-scheer](https://github.com/trevor-scheer)! - Use extensions for all imports to accommodate TS users using moduleResolution: "nodenext" - [#6846](#6846) [`2cab8f785`](2cab8f7) Thanks [@trevor-scheer](https://github.com/trevor-scheer)! - Ensure executionDidEnd hooks are only called once (when they throw) Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
When installing
@apollo/serverv4 in a TS project using"moduleResolution": "nodenext", TS throws a bunch of errors about how Apollo Server's imports need to use extensions. Specifically, we omitted extensions from all type-only imports since they didn't seem to be needed, but it's not hurting anything to add them and some use cases require it.Fixes #6730