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

type: module breaking webpack compilation #523

Closed
bazineta opened this issue Jul 15, 2024 · 6 comments · Fixed by #525
Closed

type: module breaking webpack compilation #523

bazineta opened this issue Jul 15, 2024 · 6 comments · Fixed by #525
Assignees
Milestone

Comments

@bazineta
Copy link

Post 3.4.1, the top-level package.json has been changed to have type: module.

Our host environment is ESM as well, and as such we import jquery-migrate. Unfortunately, with the type defined as module, the CommonJS/AMD wrapper is falling through to browser globals when compiled, and we don't have browser globals for anything, jquery included, and thus we're failing at runtime.

Removing type: module from the 3.5.0 package.json does resolve the issue.

Using webpack 5.93.0, jquery 3.7.1, jquery-migrate 3.5.0. Backing off to 3.4.1 resolves the issue, as that release did not have type:module present.

@mgol
Copy link
Member

mgol commented Jul 15, 2024

Thanks for the report. This regressed in #512 (cc @timmywil, we need to take this into account in projects that don't have separate *-dist repos). I wonder if we can just add a simple package.json in dist as we have in jQuery:

{
  "type": "commonjs"
}

That should resolve your issue; I wonder if there are other breaking consequences of this change, though.

@mgol mgol added this to the 3.5.1 milestone Jul 15, 2024
@mgol mgol self-assigned this Jul 15, 2024
@mgol mgol added the Discuss in Meeting Reserved for Issues and PRs that anyone would like to discuss in the weekly meeting. label Jul 15, 2024
@mgol
Copy link
Member

mgol commented Jul 15, 2024

BTW, we'd catch this if we had any Node-based or bundler tests. The Node ones are the easiest but Migrate doesn't really support Node.js.

mgol added a commit to mgol/jquery-migrate that referenced this issue Jul 15, 2024
PR jquerygh-512 added the `"type": "module"` field to the top-level `package.json`,
making Node-based workflows fail when trying to require Migrate as it's not
exposed as ESM. To fix this, add a small `package.json` with just
`"type": "commonjs"`. Also, add Node.js smoke tests as the simplest way to
verify this change.

Fixes jquerygh-523
Ref jquerygh-512
mgol added a commit to mgol/jquery-migrate that referenced this issue Jul 15, 2024
PR jquerygh-512 added the `"type": "module"` field to the top-level `package.json`,
making Node-based workflows fail when trying to require Migrate as it's not
exposed as ESM. To fix this, add a small `package.json` with just
`"type": "commonjs"`. Also, add Node.js smoke tests as the simplest way to
verify this change.

Fixes jquerygh-523
Ref jquerygh-512
@mgol
Copy link
Member

mgol commented Jul 15, 2024

PR: #525

@bazineta
Copy link
Author

Your fixes come faster than my testing 😉. Adding the described package.json in dist does indeed resolve the situation in our case.

@mgol
Copy link
Member

mgol commented Jul 15, 2024

Thanks for confirming!

@mgol mgol removed the Discuss in Meeting Reserved for Issues and PRs that anyone would like to discuss in the weekly meeting. label Jul 16, 2024
@mgol mgol closed this as completed in #525 Jul 16, 2024
@mgol mgol closed this as completed in 785943d Jul 16, 2024
@mgol
Copy link
Member

mgol commented Jul 17, 2024

Fixed in jQuery Migrate 3.5.2.

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 a pull request may close this issue.

2 participants