all: change approach to extensions on imports #716
Merged
Conversation
This file contains 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
allisonkarlitskaya
force-pushed
the
revisit-extensions
branch
2 times, most recently
from
7c2bd1b
to
5f6ad52
Compare
It's currently not possible to run ./build.js under node-22, which is a shame, because we'll want to be able to do that soon (due to upcoming TypeScript support, which will let us stop using tsx and its bundled copy of esbuild, simplifying builds on arm64). Reading the developments on the addition of TypeScript support to Node, it seems to be the case that TypeScript is somewhat isolated in their opinion on the correct approach for handling extensions on import statements. Indeed, node-22 will outright break if we don't use extensions correctly. Add back the extensions that we removed from the files in our build process. Change our use of the eslint rule to *require* extensions, rather than forbid them, and update our code accordingly. Updating our bundled code is not strictly necessary (and we could just drop the eslint rule entirely) but let's be consistent. Add an option to our tsconfig to tell TypeScript that we want to do things node's way.
allisonkarlitskaya
force-pushed
the
revisit-extensions
branch
from
5f6ad52
to
7feff84
Compare
Modern versions of node require `with { type: 'json' };` on JSON
imports. `tsc` doesn't like this with its default module system (which
is commonjs) so use "preserve", which is the recommended setting for
bundler setups, anyway.
Also add some `import type` vs. `import` cleanups: I used to think this
was excessively pedantic, but getting this right is necessary for using
`--experimental-transform-types`.
With these changes it's possible to run the build system under nodejs-22
as is currently available in Fedora. It's also possible to run it with
a currently nightly build of nodejs, without tsx:
node-22 --experimental-transform-types ./build.js
|
Some background for the discussions around this change: |
martinpitt
approved these changes
Aug 20, 2024
I guess that explains why the builds haven't been picked up for over an hour... |
allisonkarlitskaya
merged commit 093d865
into
cockpit-project:main
13 of 25 checks passed
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.
It's currently not possible to run ./build.js under node-22, which is a shame, because we'll want to be able to do that soon (due to upcoming TypeScript support, which will let us stop using tsx and its bundled copy of esbuild, simplifying builds on arm64).
Reading the developments on the addition of TypeScript support to Node, it seems to be the case that TypeScript is somewhat isolated in their opinion on the correct approach for handling extensions on import statements. Indeed, node-22 will outright break if we don't use extensions correctly.
Add back the extensions that we removed from the files in our build process. Change our use of the eslint rule to require extensions, rather than forbid them, and update our code accordingly. Updating our bundled code is not strictly necessary (and we could just drop the eslint rule entirely) but let's be consistent.
Add an option to our tsconfig to tell TypeScript that we want to do things node's way.