Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Build ESM artifacts with Node16 module resolution #599

Closed
timostamm opened this issue Oct 27, 2023 · 2 comments · Fixed by #754
Closed

Build ESM artifacts with Node16 module resolution #599

timostamm opened this issue Oct 27, 2023 · 2 comments · Fixed by #754
Labels
Cleanup Cleanup tasks

Comments

@timostamm
Copy link
Member

We currently build ESM artifacts with module resolution "Node", which is the Node.js v10 resolution algorithm.

We should switch to "Node16", which would have caught #586.
@timostamm timostamm added the Cleanup Cleanup tasks label Oct 27, 2023
@timostamm timostamm mentioned this issue Jan 31, 2024
Merged
@timostamm
Copy link
Member Author

Another extension-less import slipped through: #690

@timostamm timostamm mentioned this issue Mar 12, 2024
Merged
@timostamm
Copy link
Member Author

I just tested this in the v2 branch:

  • set moduleResolution: Node16 and module: Node16 in tsconfig.base.json
  • adjust all build:esm and build:cjs scripts (the latter needs --module commonjs --verbatimModuleSyntax false --moduleResolution node10, the former needs nothing)
  • add node10 fallback file for subpath exports (currently only protoplugin/ecmascript)

So far it passes attw and some manual tests.

@smaye81 smaye81 mentioned this issue Mar 19, 2024
Merged
smaye81 added a commit that referenced this issue Mar 20, 2024
@smaye81
Build ESM artifacts with Node16 moduleResolution (#754)
a0ba45a 
Fixes #599 

This switches all packages to build ESM artifacts with Node16 module
resolution. In doing so, it turns on Node16 for both `moduleResolution`
and `module` by default throughout the repo. It also turns on
`verbatimModuleSyntax` by default (which is why the need for all the
`import type {...}` changes).

For CJS artifacts, the exceptions for `moduleResolution node10` and
`verbatimModuleSyntax false` are now specified in the command script.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Cleanup Cleanup tasks
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Build ESM artifacts with Node16 moduleResolution bufbuild/protobuf-es
1 participant
@timostamm