/ember-cli-babel

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

Fix presets array bug #224

Merged
merged 1 commit into from May 26, 2018

Conversation

Reviewers

@rwjblue rwjblue

@kellyselden kellyselden

Assignees
No one assigned
Labels
bug
Projects
None yet
Milestone
No milestone
5 participants
@twokul @rwjblue @kellyselden @stefanpenner @Turbo87
@twokul
Copy link
Contributor

twokul commented May 26, 2018 •
edited

In ember-cli when using DELAYED_TRANSPILATION feature, we explicitly
set disablePresetEnv flag to true. shouldRunPresetEnv is a
negation of disablePresetEnv.

Prior to this change options.presets was an array with one false
item in it (shouldRunPresetEnv was set to false) and would not let files
be transplied when DELAYED_TRANSPILATION was enabled.

Useful links:
@twokul

This comment has been minimized.

Sign in to view
Copy link
Contributor Author

twokul commented May 26, 2018

unblocks ember-cli/ember-cli#7848 and fixes ember-cli/ember-cli#7850
@twokul

This comment has been minimized.

Sign in to view
Copy link
Contributor Author

twokul commented May 26, 2018

/cc @stefanpenner and @kellyselden
@rwjblue

rwjblue reviewed May 26, 2018
View changes

index.js Outdated
@@ -221,9 +221,13 @@ module.exports = {
userPostTransformPlugins
).filter(Boolean);

options.presets = [
shouldRunPresetEnv && this._getPresetEnvPlugins(addonProvidedConfig),
]

This comment has been minimized.

Sign in to view
Copy link
@rwjblue

rwjblue May 26, 2018

Member

Would it be easier to add .filter(Boolean) here instead?

This comment has been minimized.

Sign in to view
Copy link
@twokul

twokul May 26, 2018

Author Contributor

I think the condition is easier to read than &&

This comment has been minimized.

Sign in to view
Copy link
@kellyselden

kellyselden May 26, 2018

Member

I think I agree with @rwjblue, I think the consistency with the above .filter(Boolean) would be better.

This comment has been minimized.

Sign in to view
Copy link
@twokul

twokul May 26, 2018

Author Contributor

peer pressure 😝
@kellyselden

kellyselden approved these changes May 26, 2018
View changes

@twokul
Fix presets array bug 
In `ember-cli` when using `DELAYED_TRANSPILATION` feature, we explicitly
set `disablePresetEnv` flag to `true`. `shouldRunPresetEnv` is a
negation of `disablePresetEnv`.

Prior to this change `options.presets` was an array with one `false`
item in it (`shouldRunPresetEnv` was set to `false`) and would let files
be transplied when `DELAYED_TRANSPILATION` was enabled.

Useful links:

+ https://github.com/ember-cli/ember-cli/blob/2e0cade64c4698cc48b1fdc20fda2219e63cc973/lib/models/addon.js#L256
Loading status checks…
18c751d

@twokul twokul force-pushed the twokul:fix branch from af3c2ac to 18c751d May 26, 2018

@twokul

This comment has been minimized.

Sign in to view
Copy link
Contributor Author

twokul commented May 26, 2018

@rwjblue @kellyselden comment addressed, I can't merge so w/e you're ready 😺
@rwjblue

This comment has been minimized.

Sign in to view
Copy link
Member

rwjblue commented May 26, 2018

I’ll get it released in an hour or so...

@kellyselden kellyselden merged commit 194c847 into babel:master May 26, 2018
2 checks passed

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@twokul twokul deleted the twokul:fix branch May 26, 2018

@kellyselden

This comment has been minimized.

Sign in to view
Copy link
Member

kellyselden commented May 26, 2018

Released in v6.14.1

@Turbo87 Turbo87 added the bug label May 28, 2018

@stefanpenner

This comment has been minimized.

Sign in to view
Copy link
Member

stefanpenner commented on node-tests/addon-test.js in 18c751d May 30, 2018

thanks for the test!
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment