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

chore: set type to commonjs in config-loader package.json #2794

Merged
merged 3 commits into from
Aug 30, 2024

Conversation

web-padawan
Copy link
Contributor

@web-padawan web-padawan commented Aug 29, 2024

What I did

Fixes #2793

Set type to commonjs to avoid test failure due to Node 22.7 experimental module syntax detection.

Copy link

changeset-bot bot commented Aug 29, 2024

🦋 Changeset detected

Latest commit: a04ff38

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@web/config-loader Patch

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

@web-padawan web-padawan force-pushed the test/config-loader-no-detect-module branch from 7b88d4d to dc346ee Compare August 29, 2024 11:48
@@ -19,7 +19,7 @@
},
"scripts": {
"build": "tsc",
"test:node": "mocha test/**/*.test.js --reporter dot",
"test:node": "mocha test/**/*.test.js --reporter dot --no-experimental-detect-module",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this roughly the same as adding a "type": "commonjs" entry?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, adding "type": "commonjs" works too. I can add it if you think it's a better solution.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not 100% sure where this package fits in the ecosystem, but doing so would prevent any consumers from having a similar issue, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Westbrook The package is used by @web/dev-server and @web/test-runner to load respective configs.

Updated to use "type": "commonjs" and added a changeset. I think it's fine to use patch release for this.

@web-padawan web-padawan changed the title chore: disable experimental module syntax detection in test chore: set type to commonjs in config-loader package.json Aug 30, 2024
Copy link
Member

@Westbrook Westbrook left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:

@web-padawan web-padawan removed the request for review from thepassle August 30, 2024 11:25
@web-padawan web-padawan merged commit 3845d54 into master Aug 30, 2024
6 of 7 checks passed
@web-padawan web-padawan deleted the test/config-loader-no-detect-module branch August 30, 2024 11:25
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.

Config loader: test fails since Node v22.7.0
2 participants