- Notifications
You must be signed in to change notification settings - Fork361
add support for amd options#663
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:master
Are you sure you want to change the base?
Uh oh!
There was an error while loading.Please reload this page.
Conversation
I'm a little worried about these AMD-specific parameters. Out of curiosity - did you try using |
hanai commentedJun 18, 2020 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
They are not the same. in with name=moda typeofdefine==='function'&&define.amd ?define(factory) :(global=global||self,global.moda=factory()); with amd.id=modb typeofdefine==='function'&&define.amd ?define('modb',factory) :(global=global||self,global.mod=factory()); with both typeofdefine==='function'&&define.amd ?define('modb',factory) :(global=global||self,global.moda=factory()); |
As a thought, what about having an |
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
If possible, I'd love to get a better understanding of why named define is useful. I've written a few AMD loaders in the past and only ever needed named defines or custom define functions as a workaround. |
Co-authored-by: Jason Miller <developit@users.noreply.github.com>
Co-authored-by: Jason Miller <developit@users.noreply.github.com>
In case of |
@hanai We feel that adding this feature adds a bit more complex than we want to microbundle. For your use-case, it would make more sense to go with rollup directly instead of microbundle. We do see value in adding support for creating named amd modules. If you could refactor this PR to support the Thanks for understanding and contributing to microbundle! |
rart commentedNov 11, 2020
@wardpeet@developit I'm running into this issue too — i.e. the lack of support for the amd rollup options. I was thinking submitting a PR and found this one. The The amd.id is pretty important since there are certain module loaders that don't support anonymous definitions. Also, when writing a custom module loader, the id that comes to the define function is useful in countless ways — up to the lib developer. Supporting these options would be suuuuper simple — could be as simple as adding outputOptions:{ ...// if options.amdId or options.amdDefine is undefined rollup takes it as not supplied and behaves as defaultamd:{id:options.amdId,define:options.amdDefine}, ...}
As an example, you may have the define function under a certain namespace (e.g. myCompany.define) so amd.define = myCompany.define would give you the right output, otherwise it will always call define on the global object and fail if it isn't there. I'm happy to submit a PR once I hear back from you guys that it would be accepted. Please suggest approach too to minimize PR revision back-and-forth. |
bensalilijames commentedFeb 14, 2022
Would be keen to see this! Running into this when packaging a library for use in Decentraland, which requires named AMD modules with the ID being the same as the NPM package name. FWIW, I'd also be keen to have the flexibility to separately define the Understand that this increases the surface area of microbundle, though I think it's a worthwhile addition that unlocks an edge case, without requiring authors to eject from microbundle and roll their own rollup config. Would you be open to this PR as-is if I resolved the merge conflicts? What do you think? Thank you! 🙌 |
othree commentedJul 11, 2022 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
I have the same issue. If the module is un-named. There will be an issue working with the require.js. My use case is we are writing 3rd party widget. Then using our script on a site is using require.js already(and their root script is un-named too) and will see this error. |
add support for amd options
https://rollupjs.org/guide/en/#outputamd