Can this please (optionally) ignore UMD wrapped modules

[gillyspy]

It would be great if there was a way to tell the transform to ignore UMD wrapped modules.

We deploy to an environment that uses AMD but historically our testing has been using requirejs in node. We're switching to this because it's embedded in another package that will make our lives easier overall, buuuuut when we require modules written as below it fails

it would be great if no matter which type of module we are passing it would survive.

The pattern we have is something like this (grabbed this from one of my templates):

((define) => {
  define([], () => {
    const exports = {};
   //write your code here

    return exports;
  });
})(
  /**
   * @description UMD
   * @param {Array<strings>} m list of modules
   * @param {function} cb callback for define
   * @return {undefined}
   */
  (m, cb /* jshint -W014, -W030 */) => {
    const mod = module.exports;
    // eslint-disable-next-line no-unused-expressions,implicit-arrow-linebreak
    (typeof define === 'function')
      ? define(m, cb)
      : Object.assign(mod, cb(...m.map(require)));
  }
);

I'd be happy with a comment declaration if i had to as a quick starter.

[msrose]

Hi @gillyspy, thanks for opening the issue. Couple of clarifying questions:

• "our testing has been using requirejs in node": does this mean you're using RequireJS in Node.js, or just Node's built-in require?
• Can you please provide the full error that you're seeing for the example module?

[msrose]

Additionally, the restrictToTopLevelDefine options is worth toggling to see if anything changes for your UMD modules. Check out https://github.com/msrose/babel-plugin-transform-amd-to-commonjs#options for more info.

[gillyspy]

hey, thanks for the quick reply. wow. 🙏🏻

the deploy target system uses requirejs (or a customized AMD loader) .

depending on what i'm testing I'm using requirejs or require. I prefer require when i can get it so that jest can mock (which uses hoisting) and the environment context doesn't need to be modified (known issue with requirejs not knowing what global is with Jest).

in the above UMD-stle module pattern I can load with either. Many modules, including the native modules, legacy modules are written in AMD style.

in short... we test with require using modules that are expected to be transformed regardless of what they are.

The challenge here is in testing in Node (using Jest incidentally). The other package is what is calling your transform. It is not using requirejs (in node)-- internally it is calling your transform first.

So, If your transform was able to ignore modules (like my pattern above) then I wouldn't have to jump through so many hoops to test scenarios where AMD-only modules are mixed with UMD modules (let alone mixed with CommonJS modules, but that should actually work too of course cuz that's what UMD is doing)

i'll try that option but i'm not optimistic.. my hypothesis is that there is only a top level here -- it's just not one this babel plugin recognizes. also, not sure where yet to try it cuz it's kind of of buried in that oracle package (link above)

[msrose]

Okay I'm having trouble wrapping my head around the use case exactly, and it complicates things that you don't have the ability to configure the transform directly, but regardless I think the request to have a way to exclude modules (e.g. via a comment at the top of the file), is a reasonable one. I should be able to pick this up this weekend, but if you would like to pick it up yourself I would happily review a pull request. Please let me know if you decide to do so so we don't duplicate work.

[msrose]

@gillyspy #303 has been merged which gives an escape hatch to let you ignore modules by adding /* transform-amd-to-commonjs-ignore */ at the top of the file. I believe this should help fix your problem for specific modules.

It might make sense to do more testing with UMD modules at some point and add additional handling to ignore them specifically -- if anyone else has more examples of modules that need ignoring, please post on this thread and I'll reopen this issue.