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 option babelifyDeps #422

Merged
merged 1 commit into from
Apr 13, 2018
Merged

Conversation

aknuds1
Copy link
Contributor

@aknuds1 aknuds1 commented Feb 13, 2018

Choose one: This is a 🙋 feature

I add the option babelifyDeps to control whether the scripts method applies the babelify transform to dependencies or not. I need this since calling Babel on my dependency tree slows down my build by about 5-6 times and at least one of my dependencies (underscore.js) also breaks from this.

Note: I'm still unsure whether babelifyDeps should be true by default, since it's an intrusive feature that easily breaks or simply slows down your build significantly. Personally I would have it off by default.

Checklist

  • tests pass
  • tests and/or benchmarks are included
  • documentation is changed or added

Context

See #423.

Semver Changes

I would recommend a minor version upgrade, since this adds an option.

@aknuds1 aknuds1 force-pushed the optional-babelify-deps branch 2 times, most recently from f7e2e7c to e623c38 Compare February 15, 2018 10:24
@goto-bus-stop
Copy link
Member

IMO we should remove it entirely in 10.0 … or figure out a way to make it only compile stuff that is not ES5 compatible (maybe with https://www.npmjs.com/package/is-es-version and aggressive caching), or do a dual bundle thing—one with modern syntax and one with ES5.

In the mean time this is probably a reasonable compromise even though it adds an option.

@goto-bus-stop
Copy link
Member

will merge both this and #455 soon then and release as minor

@goto-bus-stop goto-bus-stop merged commit a4306c5 into choojs:master Apr 13, 2018
gmaclennan added a commit to gmaclennan/bankai that referenced this pull request Sep 9, 2019
Running babel on deps causes issues with some deps, and choojs#422 fixes this, but there is no way to set this option from the command line.

This PR marks the `babelifyDeps` CLI option as a bool, so that you can pass `--babelifyDeps=false` as an option from the CLI.
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.

None yet

2 participants