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

Add --out-file-extension option to babel-cli #9144

Merged
merged 6 commits into from Jan 10, 2020

Conversation

@eps1lon
Copy link
Contributor

eps1lon commented Dec 7, 2018

Q                       A
Fixed Issues? Fixes #6222
Patch: Bug Fix? No
Major: Breaking Change? No
Minor: New Feature? Yes
Tests Added + Pass? Yes
Documentation PR Link
Any Dependency Changes?
License MIT

Webpack 4 automatically prioritizes foo.mjs over foo.js when resolving import foo from './foo'. With this change we can publish a flat package with esmodules and commonjs side by side that allows imports such as import foo from package/bar that webpack can easily tree shake with help of esmodules.

Settled on out-file-extension to be consistent with other flags (out-dir, out-file etc.)

Alternatives for flag name:

  • --output-file-extension
  • --with-file-extension

Should I add a warning if both --keep-file-extension and --use-file-extension are used?

Yeah, it should throw.

It throws now

@ljharb

This comment has been minimized.

Copy link
Member

ljharb commented Dec 8, 2018

It might be nicer to allow the config itself specify an output extension - then we could run twice, one with each config, to output for each extension (each of which has a different set of target environments)

@eps1lon

This comment has been minimized.

Copy link
Contributor Author

eps1lon commented Dec 8, 2018

@ljharb I think so too so that you could use a single cli command and change the output extension based on some environment variable.

I think this is a better starting point though since --keep-file-extension is also a cli flag.

@karlhorky

This comment has been minimized.

Copy link

karlhorky commented Dec 8, 2018

Small nit in the PR description:

Webpack 4 automatically prioritizes foo.mjs over foo.js when resolving import foo from './bar'

Shouldn't the filenames be bar.mjs and bar.js (because of the path ./bar in the import example)?

@eps1lon eps1lon force-pushed the eps1lon:feat/cli/flag-use-extension branch from 6b4ca53 to cce3797 Feb 26, 2019
@babel-bot

This comment has been minimized.

Copy link
Collaborator

babel-bot commented Feb 26, 2019

Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/11285/

@nicolo-ribaudo

This comment has been minimized.

Copy link
Member

nicolo-ribaudo commented Feb 28, 2019

Should I add a warning if both --keep-file-extension and --use-file-extension are used?

Yeah, it should throw.

WDYT about naming it --out-file-extension, for consistency with --out-file and --out-dir?

@eps1lon eps1lon force-pushed the eps1lon:feat/cli/flag-use-extension branch from cce3797 to e12d5e5 Feb 28, 2019
@eps1lon

This comment has been minimized.

Copy link
Contributor Author

eps1lon commented Feb 28, 2019

@nicolo-ribaudo

  • refactored file extension logic (looked very brittle to me, path module already has everything we need: 680cab6
  • renamed use-file-extension -> --out-file-extension: 003d658
  • added error if both flags are used (do test these?): e12d5e5
@eps1lon eps1lon changed the title Add --use-file-extension option to babel-cli Add --out-file-extension option to babel-cli Feb 28, 2019
@eps1lon eps1lon force-pushed the eps1lon:feat/cli/flag-use-extension branch from 9c84bae to d5dc562 Feb 28, 2019
@eps1lon

This comment has been minimized.

Copy link
Contributor Author

eps1lon commented Feb 28, 2019

@nicolo-ribaudo Done d5dc562. Initially forgot to add a trailing newline to stderr.txt and options.json. Added them later and squashed but github doesn't pick it up.

Copy link
Member

nicolo-ribaudo left a comment

It seems that it is working anyway?

@IvanGoncharov

This comment has been minimized.

Copy link

IvanGoncharov commented Mar 26, 2019

I would love to see this PR merged since it will allow us to remove ugly shell scripts from our package.json:
https://github.com/graphql/graphql-js/blob/0b5e95556ef8334416a1dcb97084c1cc0a21ff5e/package.json#L41-L42
and make our project more Windows-friendly.

ATM we have a choice of either waiting until this feature gets merged or drop babel-cli and write our build scripts using babel-core.
Can someone from Babel team please clarify the status of this PR?

@eps1lon eps1lon force-pushed the eps1lon:feat/cli/flag-use-extension branch from d5dc562 to 34db0a0 Mar 26, 2019
@jaydenseric

This comment has been minimized.

Copy link

jaydenseric commented May 28, 2019

Is there something blocking a merge?

Really looking forward to this shipping; so we can output separate .mjs and .cjs files with package scripts like this:

{
  "prepare:mjs": "BABEL_ESM=1 babel src -d . --out-file-extension .mjs",
  "prepare:cjs": "babel src -d . --out-file-extension .cjs"
}
@eps1lon eps1lon force-pushed the eps1lon:feat/cli/flag-use-extension branch from 34db0a0 to dfa4a31 Jul 24, 2019
@eps1lon eps1lon force-pushed the eps1lon:feat/cli/flag-use-extension branch from dfa4a31 to 1415c95 Aug 5, 2019
Copy link
Contributor

JLHwung left a comment

Thanks! Docs is needed anyway.

@eps1lon

This comment has been minimized.

Copy link
Contributor Author

eps1lon commented Nov 15, 2019

Thanks! Docs is needed anyway.

See babel/website#2128

@eps1lon eps1lon mentioned this pull request Nov 19, 2019
0 of 9 tasks complete
@JLHwung JLHwung removed the PR: Needs Docs label Nov 20, 2019
@manuelbieh

This comment has been minimized.

Copy link

manuelbieh commented Dec 11, 2019

Any rough estimation on when this will be merged? Is there anything left to be done I could help with? I would really love to see this feature merged because I'm currently trying to get native ESModules working with Node and Node requires .mjs as file extension for ESModules unless you set "type":"module" in your package.json.

@eps1lon

This comment has been minimized.

Copy link
Contributor Author

eps1lon commented Dec 11, 2019

I think this is just waiting for the next minor babel release.

Looking at the 7.8 milestone and what is ready to be shipped it seems close enough.

On the bright side: This PR could celebrate its 1 year anniversary 😄

@nicolo-ribaudo nicolo-ribaudo merged commit 3af02f6 into babel:master Jan 10, 2020
5 checks passed
5 checks passed
babel/repl REPL preview is available
Details
buildsize No significant change
Details
ci/circleci Your tests passed on CircleCI!
Details
codecov/project 87.85% (target 80%)
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@tbranyen

This comment has been minimized.

Copy link

tbranyen commented Jan 10, 2020

Thanks for landing this, much appreciated!

@FezVrasta

This comment has been minimized.

Copy link

FezVrasta commented Jan 13, 2020

How can a plugin read the --out-file-extension value? I'm trying to add support for it on the babel-plugin-add-import-extension plugin.

@nicolo-ribaudo

This comment has been minimized.

Copy link
Member

nicolo-ribaudo commented Jan 16, 2020

@FezVrasta It's not currently possible. The options of the tool used to call Babel (i.e. @babel/cli, babel-loader, rollup-plugin-babel, etc) are unrelated to the options passed to Babel itself and to the plugins.

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

Successfully merging this pull request may close these issues.

You can’t perform that action at this time.