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.

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

compute-baseline: Use JSON import attribute when importing BCD #1051

Merged
merged 2 commits into from
Jun 11, 2024

Conversation

ddbeck
Copy link
Collaborator

@ddbeck ddbeck commented May 6, 2024

I think this will fix the issue reported by @tidoust in #967 (comment).

This change will require one of Node.js v16.14.0+ (see #1051 (comment)), v18.20.0+, or v20.10.0+.

If you want to test this immediately, do the following:

  1. Check out this PR's branch.
  2. Run npm pack --workspace compute-baseline. A .tgz file for the package is built.
  3. In your project, install the .tgz file. For example, run npm install ../path/to/compute-baseline-0.1.0.tgz.

@tidoust
Copy link
Member

tidoust commented May 6, 2024

That seems a good start but there are other import statements that need the same treatment at the top of other files: browser.ts, feature.ts, supportStatements.ts, release.ts, typeUtils.ts, walk.ts.

Also, I don't think that will work with v16.x (the syntax then used assert instead of with) but that's probably fine?

@ddbeck
Copy link
Collaborator Author

ddbeck commented May 8, 2024

That seems a good start but there are other import statements that need the same treatment at the top of other files: browser.ts, feature.ts, supportStatements.ts, release.ts, typeUtils.ts, walk.ts.

The other imports are TypeScript type imports. When TypeScript compiles, these statements are not emitted into the final .js files. Adding import attributes to those imports would have no effect on the built package.

Also, I don't think that will work with v16.x (the syntax then used assert instead of with) but that's probably fine?

Ah, I made a mistake in my original description—import attributes were never backported to Node 16. I think this is OK though, since v16 has been EOL since September of last year.

@ddbeck ddbeck requested a review from foolip June 6, 2024 11:07
@ddbeck ddbeck merged commit 8f12268 into web-platform-dx:main Jun 11, 2024
3 checks passed
@ddbeck ddbeck deleted the import-attrs-bcd branch June 11, 2024 15:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants