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

module: do not warn for typeless package.json when there isn't one #54045

Merged
merged 1 commit into from
Jul 29, 2024
Merged

module: do not warn for typeless package.json when there isn't one #54045

merged 1 commit into from
Jul 29, 2024

Conversation

joyeecheung
Copy link
Member

It was intended that warnings should only be emitted for an existing package.json without a type. This fixes a confusing warning telling users to update /package.json when there are no package.json on the lookup path at all, like this:

[MODULE_TYPELESS_PACKAGE_JSON] Warning: ... parsed as an ES module because module syntax was detected; to avoid the performance penalty of syntax detection, add "type": "module" to /package.json

Drive-by: update the warning message to be clear about reparsing and make it clear what's actionable.

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/loaders

@nodejs-github-bot nodejs-github-bot added esm Issues and PRs related to the ECMAScript Modules implementation. needs-ci PRs that need a full CI run. labels Jul 25, 2024
jsumners-nr
jsumners-nr approved these changes Jul 25, 2024
View reviewed changes
Copy link

@jsumners-nr jsumners-nr left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@joyeecheung
module: do not warn for typeless package.json when there isn't one
cf52a8b 
It was intended that warnings should only be emitted for an
existing package.json without a type. This fixes a confusing
warning telling users to update /package.json when there are
no package.json on the lookup path at all, like this:

[MODULE_TYPELESS_PACKAGE_JSON] Warning: ... parsed as an ES module
because module syntax was detected; to avoid the performance penalty
of syntax detection, add "type": "module" to /package.json

Drive-by: update the warning message to be clear about
reparsing and make it clear what's actionable.
@aduh95 aduh95 added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. request-ci Add this label to start a Jenkins CI on a PR. labels Jul 25, 2024
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jul 25, 2024
@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/60630/

mcollina
mcollina approved these changes Jul 28, 2024
View reviewed changes
Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

lgtm

@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/60702/

@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/60723/

@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/60726/

@joyeecheung joyeecheung added the commit-queue Add this label to land a pull request using GitHub Actions. label Jul 29, 2024
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Jul 29, 2024
@nodejs-github-bot nodejs-github-bot merged commit be7a0c5 into nodejs:main Jul 29, 2024
59 checks passed
@nodejs-github-bot
Copy link
Collaborator

Landed in be7a0c5

targos pushed a commit that referenced this pull request Jul 30, 2024
@joyeecheung @targos
module: do not warn for typeless package.json when there isn't one
343b3b5 
It was intended that warnings should only be emitted for an
existing package.json without a type. This fixes a confusing
warning telling users to update /package.json when there are
no package.json on the lookup path at all, like this:

[MODULE_TYPELESS_PACKAGE_JSON] Warning: ... parsed as an ES module
because module syntax was detected; to avoid the performance penalty
of syntax detection, add "type": "module" to /package.json

Drive-by: update the warning message to be clear about
reparsing and make it clear what's actionable.

PR-URL: #54045
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@RafaelGSS RafaelGSS mentioned this pull request Jul 30, 2024
Merged
@RafaelGSS RafaelGSS added dont-land-on-v18.x PRs that should not land on the v18.x-staging branch and should not be released in v18.x. dont-land-on-v20.x PRs that should not land on the v20.x-staging branch and should not be released in v20.x. dont-land-on-v22.x PRs that should not land on the v22.x-staging branch and should not be released in v22.x. labels Aug 5, 2024
@RafaelGSS
Copy link
Member

Adding dont-land labels until #53619 flags are removed too.

@marco-ippolito marco-ippolito removed the dont-land-on-v22.x PRs that should not land on the v22.x-staging branch and should not be released in v22.x. label Aug 26, 2024
marco-ippolito pushed a commit to marco-ippolito/node that referenced this pull request Aug 26, 2024
@joyeecheung @marco-ippolito
module: do not warn for typeless package.json when there isn't one
0c53074 
It was intended that warnings should only be emitted for an
existing package.json without a type. This fixes a confusing
warning telling users to update /package.json when there are
no package.json on the lookup path at all, like this:

[MODULE_TYPELESS_PACKAGE_JSON] Warning: ... parsed as an ES module
because module syntax was detected; to avoid the performance penalty
of syntax detection, add "type": "module" to /package.json

Drive-by: update the warning message to be clear about
reparsing and make it clear what's actionable.

PR-URL: nodejs#54045
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@marco-ippolito marco-ippolito added the backport-open-v22.x Indicate that the PR has an open backport label Aug 26, 2024
marco-ippolito pushed a commit to marco-ippolito/node that referenced this pull request Aug 26, 2024
@joyeecheung @marco-ippolito
module: do not warn for typeless package.json when there isn't one
613842d 
It was intended that warnings should only be emitted for an
existing package.json without a type. This fixes a confusing
warning telling users to update /package.json when there are
no package.json on the lookup path at all, like this:

[MODULE_TYPELESS_PACKAGE_JSON] Warning: ... parsed as an ES module
because module syntax was detected; to avoid the performance penalty
of syntax detection, add "type": "module" to /package.json

Drive-by: update the warning message to be clear about
reparsing and make it clear what's actionable.

PR-URL: nodejs#54045
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
marco-ippolito pushed a commit to marco-ippolito/node that referenced this pull request Aug 31, 2024
@joyeecheung @marco-ippolito
module: do not warn for typeless package.json when there isn't one
296b4b1 
It was intended that warnings should only be emitted for an
existing package.json without a type. This fixes a confusing
warning telling users to update /package.json when there are
no package.json on the lookup path at all, like this:

[MODULE_TYPELESS_PACKAGE_JSON] Warning: ... parsed as an ES module
because module syntax was detected; to avoid the performance penalty
of syntax detection, add "type": "module" to /package.json

Drive-by: update the warning message to be clear about
reparsing and make it clear what's actionable.

PR-URL: nodejs#54045
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
marco-ippolito pushed a commit to marco-ippolito/node that referenced this pull request Sep 2, 2024
@joyeecheung @marco-ippolito
module: do not warn for typeless package.json when there isn't one
ff308cd 
It was intended that warnings should only be emitted for an
existing package.json without a type. This fixes a confusing
warning telling users to update /package.json when there are
no package.json on the lookup path at all, like this:

[MODULE_TYPELESS_PACKAGE_JSON] Warning: ... parsed as an ES module
because module syntax was detected; to avoid the performance penalty
of syntax detection, add "type": "module" to /package.json

Drive-by: update the warning message to be clear about
reparsing and make it clear what's actionable.

PR-URL: nodejs#54045
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@joyeecheung joyeecheung removed dont-land-on-v20.x PRs that should not land on the v20.x-staging branch and should not be released in v20.x. backport-open-v22.x Indicate that the PR has an open backport labels Sep 5, 2024
@joyeecheung
Copy link
Member Author

joyeecheung commented Sep 5, 2024 •
edited
Loading

I think this should've been released before unflagging is done in v22.7.0, instead of waiting for that to happen first, because the funny error about missing type field in /package.json is showing up now when you try to load a loose esm.js:

$ node esm.js
16
(node:437463) [MODULE_TYPELESS_PACKAGE_JSON] Warning: file:///home/ubuntu/node/esm.js parsed as an ES module because module syntax was detected; to avoid the performance penalty of syntax detection, add "type": "module" to /package.json

aduh95 pushed a commit that referenced this pull request Sep 12, 2024
@joyeecheung @aduh95
module: do not warn for typeless package.json when there isn't one
b7284ed 
It was intended that warnings should only be emitted for an
existing package.json without a type. This fixes a confusing
warning telling users to update /package.json when there are
no package.json on the lookup path at all, like this:

[MODULE_TYPELESS_PACKAGE_JSON] Warning: ... parsed as an ES module
because module syntax was detected; to avoid the performance penalty
of syntax detection, add "type": "module" to /package.json

Drive-by: update the warning message to be clear about
reparsing and make it clear what's actionable.

PR-URL: #54045
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@RafaelGSS RafaelGSS mentioned this pull request Sep 16, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mcollina mcollina mcollina approved these changes

@jasnell jasnell jasnell approved these changes

@GeoffreyBooth GeoffreyBooth GeoffreyBooth approved these changes

@benjamingr benjamingr benjamingr approved these changes

@aduh95 aduh95 aduh95 approved these changes

@jsumners-nr jsumners-nr jsumners-nr approved these changes

Assignees
No one assigned
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. dont-land-on-v18.x PRs that should not land on the v18.x-staging branch and should not be released in v18.x. esm Issues and PRs related to the ECMAScript Modules implementation. needs-ci PRs that need a full CI run.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
10 participants
@joyeecheung @nodejs-github-bot @RafaelGSS @mcollina @jasnell @GeoffreyBooth @benjamingr @aduh95 @jsumners-nr @marco-ippolito