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

feat: re-transpile excluded modules when target is es5 #1

Closed

Conversation

klipstein
Copy link
Contributor

Thanks for improving the two initial versions of the rollup-plugin-swc.

I wanted to use it in a project where I partially have to transpile to ES5 (to still support IE11) and this particular project uses quite some CommonJS node dependencies which get processed via @rollup/plugin-commonjs.

The rollup-plugin commonjs then partially generates "proxy" dependencies (which look similar to this) which need to be excluded from the initial transform(code, id), because swc cannot resolve those proxy dependencies and the build would then fail with errors like this:

[!] (plugin swc) Error: error: Unterminated string constant
  |
1 | import * as commonjsHelpers from '
  |

When those node module dependencies are excluded from swc this error disappears but then those external dependencies (which mostly are ES6 = ES2015) won't be transpiled to ES5.

It probably is an edge case, but it would be great if you include it in version 3 of rollup-plugin-swc so that I can avoid creating rollup-plugin-swc4 :)

@SukkaW
Copy link
Owner

SukkaW commented Nov 17, 2021

@klipstein

Just set exclude to /node_modules/core-js then you will be able to transpile libraries under node_modules.

It is designed to let swcTransform uses rollup's transform hook so that you can transpile many files at the same time.

@klipstein
Copy link
Contributor Author

Hi @SukkaW. This did not work for me. The commonjsHelpers is included in every commonjs module that get run through rollup plugin commonjs.

@klipstein
Copy link
Contributor Author

@SukkaW I think I got closer to the issue. It seems that swc does not like the \0 escape that the rollup plugin commonjs generates: https://github.com/rollup/plugins/blob/master/packages/commonjs/src/helpers.js#L15

@SukkaW
Copy link
Owner

SukkaW commented Nov 22, 2021

@SukkaW I think I got closer to the issue. It seems that swc does not like the \0 escape that the rollup plugin commonjs generates: https://github.com/rollup/plugins/blob/master/packages/commonjs/src/helpers.js#L15

@klipstein I might have an idea. Would you mind try this one out:

  1. Install @rollup/plugin-node-resolve (npm install @rollup/plugin-node-resolve --save-dev)
  2. In your rollup.config.js, add @rollup/plugin-node-resolve at the start of your plugin array.
import { nodeResolve } from '@rollup/plugin-node-resolve';
// ...
plugin: [
  nodeResolve(),
  // Your other plugins, like swc
]

Then test if commonjsHelpers is still being duplicated.

Detailed explanation: I have searched \0 in rollup plugins' repo and here is what I found:

https://github.com/rollup/plugins/blob/02fb349d315f0ffc55970fba5de20e23f8ead881/packages/node-resolve/src/index.js#L253-L261

If your test with @rollup/plugin-node-resolve success, I could add a similar approach to this plugin as well.

@klipstein
Copy link
Contributor Author

klipstein commented Nov 22, 2021

My rollup setup is as such:

plugin: [
  nodeResolve({
      preferBuiltins: false,
      browser: true
    }),
    commonjs({
      include: ['node_modules/axios/**']
    }),
    swc(...)
]

When I replace \0 before calling transform here (https://github.com/SukkaW/rollup-plugin-swc/blob/master/src/index.ts#L107) it works:

return swcTransform(code.replace('from "\0', 'from "'), swcOption);

@klipstein
Copy link
Contributor Author

Maybe this also helps further: https://rollupjs.org/guide/en/#conventions

There it says:

If your plugin uses 'virtual modules' (e.g. for helper functions), prefix the module ID with \0. This prevents other plugins from trying to process it.

@klipstein
Copy link
Contributor Author

@SukkaW opened a new pull #2. Closing this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants