microbundle icon indicating copy to clipboard operation
microbundle copied to clipboard

add support for amd options

Open hanai opened this issue 4 years ago • 9 comments

add support for amd options

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

hanai avatar Jun 18 '20 12:06 hanai

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. The amd.define option seems okay, though I'd be tempted to omit it from the documentation and just support it as a hidden feature.

developit avatar Jun 18 '20 14:06 developit

They are not the same.

in package.json name=mod

with name=moda

    typeof define === 'function' && define.amd ? define(factory) :
    (global = global || self, global.moda = factory());

with amd.id=modb

    typeof define === 'function' && define.amd ? define('modb', factory) :
    (global = global || self, global.mod = factory());

with both

    typeof define === 'function' && define.amd ? define('modb', factory) :
    (global = global || self, global.moda = factory());

hanai avatar Jun 18 '20 14:06 hanai

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

developit avatar Jun 19 '20 22:06 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.

developit avatar Jun 19 '20 22:06 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.

In case of define is not named as define in global, such as fmd.define.

hanai avatar Jun 20 '20 04:06 hanai

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

Thanks for understanding and contributing to microbundle!

wardpeet avatar Jul 05 '20 13:07 wardpeet

@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 amdNamed option only half-solves amd.id. Rollup has those props separate, one is the name/namespace for the module and the other — the amd.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 default
  amd: {
    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.

rart avatar Nov 11 '20 16:11 rart

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 amd.id and amd.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! 🙌

bensalilijames avatar Feb 14 '22 10:02 bensalilijames

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

othree avatar Jul 11 '22 09:07 othree