Skip to content

Conversation

@cocco111
Copy link

Summary

Proposal to change exports settings in package.json
Use case: webpack with target browser, the export proposed will be always development /production (so jquery-module).
But if someone uses require('jquery') for example in old CJS packages, is more correct and compatible loading jquery.js, as it is for node target.

Checklist

@linux-foundation-easycla
Copy link

CLA Not Signed

@mgol
Copy link
Member

mgol commented Feb 10, 2024

Thanks for the PR but I don't think the proposed changes are correct. Bundlers work differently than Node and they're perfectly able to load an ESM module via a CommonJS require as well as load a CommonJS module via an ESM import. Also, it's very important to avoid loading two different copies of jQuery which is why node.import points to a special wrapper over CommonJS and not the full ESM module.

Is there any concrete issue you're facing with Webpack or just a feeling that the setup is not what you expected? I'd prefer if we focused on real issues here; I spent a lot of time thinking about this setup and discussing it with experts in the area; any changes to this setup should be a result of real demonstrable issues.

@mgol
Copy link
Member

mgol commented Feb 10, 2024

Also, please sign our CLA, it's a hard requirement for us to accept any PRs.

@cocco111
Copy link
Author

Correct, I understood problem of "two copies" just few minutes after this PR ( in fact, I returned here to close it).
The issue is this one:
import $ from 'jquery' -> $ is JQuery function, the default export
$ = require('jquery') -> $ is whole ESModule object.
To have $ function, the statement would be $ = require('jquery').default but that require is in a lot of libraries.
The reason of this behavior is how node converts ESM modules in CJS when uses the require function.
(it's Babel?)

@mgol
Copy link
Member

mgol commented Feb 10, 2024

Can you post a full test case with detailed instructions on how to run it? I don’t think we’ll be able to proceed until we have it; there are too many edge cases here to be able to act just on reading incomplete code snippets.

@cocco111
Copy link
Author

https://github.com/cocco111/jQuery4Test
If you prefer you can simply open /dist/index.html in a browser and see result in console.

@mgol
Copy link
Member

mgol commented Feb 11, 2024

Thanks for the test case, I think I understand the issue now. The fact that we started exporting more than a single thing from jQuery - apart from the default export we have $ & jQuery named tokens pointing to the same thing - also makes Webpack treat require( "jquery" ) as a way to export the full module instead of just the jQuery object...

Let me create an issue out of it. The PR as it stands is not going to work due to state duplication but we need to think how to handle this.

@mgol
Copy link
Member

mgol commented Feb 11, 2024

Issue: #5416

@cocco111
Copy link
Author

Thanks.
I agree this PR is not correct to resolve. Now close and continue on the issue

@cocco111 cocco111 closed this Feb 11, 2024
@cocco111 cocco111 deleted the patch-1 branch February 11, 2024 12:17
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 24, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants