Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

babel-plugin-transform-modules-amd wraps with `define` when `define` already wrapping #10512

Open
rafde opened this issue Oct 1, 2019 · 4 comments

Comments

@rafde
Copy link

commented Oct 1, 2019

Bug Report

Current Behavior
When a module is already wrapped with define, babel-plugin-transform-modules-amd will still wrap it with another define.

Input Code
https://babeljs.io/repl#?babili=false&browsers=ie%2011&build=&builtIns=false&spec=false&loose=false&code_lz=CYUwZglgdiAUYFcoGMAuED2VYCcQEcEI8BKAAgG8AoMsvVBHKSgXwG4qWS2g&debug=false&forceAllTransforms=false&shippedProposals=false&circleciRepo=&evaluate=false&fileSize=false&timeTravel=false&sourceType=module&lineWrap=true&presets=&prettier=true&targets=&version=7.6.2&externalPlugins=%40babel%2Fplugin-transform-modules-amd%407.5.0

define(function(require) {
  return {};
});

Current output

define([], function() {
  "use strict";

  define(function(require) {
    return {};
  });
});

Expected output

"use strict";
define(function(require) {
  return {};
});

Expected behavior/code
Don't wrap with another define if it is already wrapped.

Environment

  • Babel version(s): 7
  • Node/npm version: All supported versions
  • Monorepo: yes
  • How you are using Babel: loader

Possible Solution
Don't wrap with another define if it is already wrapped.

@babel-bot

This comment has been minimized.

Copy link
Collaborator

commented Oct 1, 2019

Hey @rafde! We really appreciate you taking the time to report an issue. The collaborators on this project attempt to help as many people as possible, but we're a limited number of volunteers, so it's possible this won't be addressed swiftly.

If you need any help, or just have general Babel or JavaScript questions, we have a vibrant Slack community that typically always has someone willing to help. You can sign-up here for an invite."

@nicolo-ribaudo

This comment has been minimized.

Copy link
Member

commented Oct 3, 2019

If that file is not an es module, you should not run a modules plugin on it.

Consider this example:

define(function(require) {
  return {};
});

export var a = 2;

It should be wrapped in a define call, because it's obviously an es module:

define(["exports"], function(_exports) {
  "use strict";

  define(function(require) {
    return {};
  });

  var a = 2;
  _exports.a = a;
});

If there aren't any export or import, it is not possible to decide if a file if a script or an ES module. For this reason, Babel must trust that the user is doing the correct thing. In your case, you are handling it as a module and thus Babel is wrapping it.

@rafde

This comment has been minimized.

Copy link
Author

commented Oct 3, 2019

define(function(require) {
  return {};
});

export var a = 2;

I'd never fathom the above code would be done since it looks like a mixture of AMD and ES. I don't think supporting the mixture should be allowed, but I suppose define isn't being assumed as ES. But how do I handle it as AMD instead of ES?

The repo I am working on has a mixture of

define(function(require) {
  return {};
});

and

import something from 'some-module';

export default {};

due to a large number of files that are using define and newer files using import, it is unfeasible to switch out to a modern syntax due to potential regression.

At least an option would be nice to include. Or direction of what I can do using Babel?

Thanks.

@rafde

This comment has been minimized.

Copy link
Author

commented Oct 4, 2019

If that file is not an es module, you should not run a modules plugin on it.

after playing with the REPL, I noticed Source Type and changed it to unambiguous and the transform didn't wrap.

I will explore https://babeljs.io/docs/en/options#sourcetype to see if this helps. I'll try to help update the docs to help with examples on it's usage. To be continued.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.