Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

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

Open
hanai wants to merge5 commits intodevelopit:master
base:master
Choose a base branch
Loading
fromhanai:support-amd-id

Conversation

hanai
Copy link

add support for amd options

https://rollupjs.org/guide/en/#outputamd

rart reacted with thumbs up emoji
@developit
Copy link
Owner

I'm a little worried about these AMD-specific parameters.

Out of curiosity - did you try using--name? It should cause AMD output to use named define instead of anonymous. Theamd.define option seems okay, though I'd be tempted to omit it from the documentation and just support it as a hidden feature.

@hanai
Copy link
Author

hanai commentedJun 18, 2020
edited
Loading

They are not the same.

inpackage.json name=mod

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());
developit and rart reacted with thumbs up emoji

@developit
Copy link
Owner

As a thought, what about having an--amdNamed boolean option that causesamd.id to be set to--name?

@developit
Copy link
Owner

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.

hanaiand others added2 commitsJune 20, 2020 12:21
Co-authored-by: Jason Miller <developit@users.noreply.github.com>
Co-authored-by: Jason Miller <developit@users.noreply.github.com>
@hanai
Copy link
Author

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.

In case ofdefine is not named asdefine in global, such asfmd.define.

@wardpeet
Copy link
Collaborator

@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 theamdNamed boolean, that would be great.

Thanks for understanding and contributing to microbundle!

@rart
Copy link

@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.

TheamdNamed option only half-solvesamd.id. Rollup has those props separate, one is the name/namespace for the module and the other — theamd.id — is used on the define call to define the module with an id.

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},  ...}

@developit: 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.

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.

@developitdevelopit added the increased scopeIncreases project scope, or is out of scope. labelDec 18, 2020
@bensalilijames
Copy link

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 theamd.id andamd.define options as in the PR currently, instead of a--amdNamed boolean. In my case, I want to have a UMD build with the browser global having a different name from the AMD ID. If they're separate options in the CLI, I then have the power to do so!

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
Copy link

othree commentedJul 11, 2022
edited
Loading

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.

Reference:https://requirejs.org/docs/errors.html#mismatch

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@developitdevelopitdevelopit left review comments

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
increased scopeIncreases project scope, or is out of scope.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

6 participants
@hanai@developit@wardpeet@rart@bensalilijames@othree

[8]ページ先頭

©2009-2025 Movatter.jp