pkg icon indicating copy to clipboard operation
pkg copied to clipboard

feat: allow to specify custom babel plugins

Open robertsLando opened this issue 2 years ago • 9 comments

This can easily be done using PKG_BABEL_PLUGINS env var

robertsLando avatar May 30 '22 13:05 robertsLando

Hmmm... We definitely don't want to have more env variables.

Additionally, any Babel plugins have to be in the dependency (node_modules) tree of pkg. It is improbable to add them all as dependencies.

Users should not run pkg on an entrypoint that's otherwise not runnable on Node.js runtime without transpilation. Users should run the transpilation first and then run pkg.

Thus, additional Babel plugins are definitely out-of-scope for pkg, and I don't think we can move forward in any way with this idea.

jesec avatar May 30 '22 22:05 jesec

@jesec In my case I needed to add 'classPrivateMethods' and it doesn't require a plugin to be installed. It is used by a module of my application. Should I add that to babel plugins by hardcoding it here so?

BTW I don't see any issue in adding env vars, they just add advanced functionalities to the application, them are not intended to be used by all users

robertsLando avatar May 31 '22 06:05 robertsLando

@jesec In my case I needed to add 'classPrivateMethods' and it doesn't require a plugin to be installed. It is used by a module of my application. Should I add that to babel plugins by hardcoding it here so?

BTW I don't see any issue in adding env vars, they just add advanced functionalities to the application, them are not intended to be used by all users

See https://github.com/vercel/pkg/pull/1249#issuecomment-1111543386.

Unless we want to diverge from this, we can't have additional Babel plugins.

I am open to discussion about this, though. It might make some sense to shift the burden of the correctness check to users. In that case, we can use preset/env with targets.node set to the latest Node supported by us.

jesec avatar May 31 '22 07:05 jesec

It might make some sense to shift the burden of the correctness check to users

yeah like I said before, If someone really want to do this it means he knows what he is doing, I mean if you set that env var you must at least have read the docs to know it exists.

robertsLando avatar May 31 '22 08:05 robertsLando

It might make some sense to shift the burden of the correctness check to users

yeah like I said before, If someone really want to do this it means he knows what he is doing, I mean if you set that env var you must at least have read the docs to know it exists.

Env variable is definitely no-go here.

jesec avatar May 31 '22 08:05 jesec

we can use preset/env with targets.node set to the latest Node supported by us.

How can we do this so?

robertsLando avatar May 31 '22 09:05 robertsLando

we can use preset/env with targets.node set to the latest Node supported by us.

How can we do this so?

Basically, we allow using any language feature, as long as it is supported by the latest Node supported by us.

jesec avatar Jun 01 '22 07:06 jesec

You mean #1648 ?

robertsLando avatar Jun 01 '22 08:06 robertsLando

You mean #1648 ?

Yes. Per https://babeljs.io/docs/en/babel-parser#latest-ecmascript-features, we don't need to specify our own plugin array.

The reliance on estree in our code is definitely an issue, and I would have to fix that.

jesec avatar Jun 01 '22 08:06 jesec

This pull-request is stale because it has been open 90 days with no activity. Remove the stale label or comment or this will be closed in 5 days. To ignore this pull-request entirely you can add the no-stale label

github-actions[bot] avatar Aug 31 '22 00:08 github-actions[bot]