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

UMD format with lodash breaks due to lodash-es substitution #759

Open
zhaoyao91 opened this issue Jul 16, 2020 · 7 comments
Open

UMD format with lodash breaks due to lodash-es substitution #759

zhaoyao91 opened this issue Jul 16, 2020 · 7 comments
Labels
kind: bug Something isn't working scope: upstream Issue in upstream dependency

Comments

@zhaoyao91
Copy link

zhaoyao91 commented Jul 16, 2020

Current Behavior

To export umd module, I would like to include all modules into the bundle, so I config the rollup

    if (config.output.format === 'umd') {
      delete config.external;
    }

but when building, it crashes:

(babel plugin) SyntaxError: /Users/bytedance/workspace/ee_web_apps-menu/node_modules/lodash-es/isBuffer.js: 'import' and 'export' may only appear at the top level (4:0)

  2 |
  3 | var isBuffer_1 = commonjsHelpers.createCommonjsModule(function (module, exports) {
> 4 | import root from './_root.js';
    | ^
  5 | import stubFalse from './stubFalse.js';
  6 |
  7 | /** Detect free variable `exports`. */

at undefined:4:0

Expected behavior

Suggested solution(s)

when building umd module, it should not rename lodash to lodash-es

Additional context

Your environment

Software Version(s)
TSDX ^0.13.2
TypeScript
Browser
npm/Yarn
Node
Operating System
@agilgur5
Copy link
Collaborator

agilgur5 commented Jul 16, 2020

when building umd module, it should not rename lodash to lodash-es

Ah yep, that's probably required for UMD. Currently the code only checks for CJS. Should be a one-liner fix here: https://github.com/formium/tsdx/blob/8b148cea34c1852f6fcf6e94a3758d7a9f46d9b2/src/babelPluginTsdx.ts#L72

EDIT: actually I'm not so sure this isn't due to an upstream bug. There's a CJS plugin for Rollup used that should convert lodash-es too. (Misinterpreted that plugin, it does the inverse) The error is coming from Babel and not Rollup, and it's complaining about location of import and not usage of import 🤔
Though I think the one-liner fix would still resolve this error.

@agilgur5 agilgur5 added kind: bug Something isn't working PR welcome labels Jul 16, 2020
@agilgur5 agilgur5 changed the title cannot use lodash and build umd format UMD format with lodash breaks due to lodash-es substitution Jul 16, 2020
@agilgur5 agilgur5 added the good first issue Good for newcomers label Jul 19, 2020
@Morphexe
Copy link

How do I disable this feature ?

@agilgur5
Copy link
Collaborator

agilgur5 commented Sep 25, 2020

@Morphexe not sure what you mean, this is a bug, not a feature.

I think one can do a partial workaround for this by configuring babelrc with a dummy entry:

module.exports = {
  // ...
  plugins: [
    // ...
    ['babel-plugin-transform-rename-import', { replacements: [{ original: '', replacement: '' }] }]
  ]
}

But conversely, that'll probably break ESM builds

@agilgur5
Copy link
Collaborator

agilgur5 commented Oct 27, 2020

So #912 (comment) revealed a similar issue that points to upstream rollup/plugins#304 which would seem to confirm my suspicion that there's a bug upstream.

Per my comment there:

That seems to be fixed in v12.0.0, but that version has an undocumented requirement on Rollup 2, which is a very breaking change slated for v0.15.0.

Will have to add a test for this in v0.15.0 to ensure it works after the upgrade.

@axedre
Copy link

axedre commented Feb 9, 2021

@Morphexe not sure what you mean, this is a bug, not a feature.

I think one can do a partial workaround for this by configuring babelrc with a dummy entry:

  // ...
  plugins: [
    // ...
    ['babel-plugin-transform-rename-import', replacements: [{ original: '', replacement: '' }]
  ]
}

But conversely, that'll probably break ESM builds

Can you post a working example please? The snippet syntax looks borked...

@agilgur5
Copy link
Collaborator

agilgur5 commented Feb 9, 2021

@axedre ah looks like it was missing a closing bracket, I updated my comment

@axedre
Copy link

axedre commented Feb 11, 2021

That still doesn't look like valid javascript. For anyone else stumbling on this, check out: https://www.npmjs.com/package/babel-plugin-transform-rename-import.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: bug Something isn't working scope: upstream Issue in upstream dependency
Projects
None yet
Development

No branches or pull requests

4 participants