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

Fixup transpilation issues with @ember/ordered-set. #5557

Merged

Conversation

rwjblue
Copy link
Member

@rwjblue rwjblue commented Aug 7, 2018

The first commit here, demonstrates the failure reported in #5556 by running the test suite in "Chrome only" mode.

Follow up commits clean things up, and ensure we test both transpiled maximally (down to IE11) and minimally.

These cases were all previously working due to transpilation from `let`
to `var`. You *cannot* reference a block scoped variable (`let` /
`const`) during the variables own initializer.
@rwjblue
Copy link
Member Author

rwjblue commented Aug 8, 2018

Just pushed another commit here that shows how important these test harness changes are. We essentially had some code that was only working due to transpilation...

@runspired
Copy link
Contributor

Thanks @rwjblue ! I've been removing those sorts of run+set situations for a while now, glad to have something that caught the rest and got us off of them :)

@runspired runspired merged commit 99d3635 into emberjs:master Aug 8, 2018
NullVoxPopuli pushed a commit to NullVoxPopuli/data that referenced this pull request Sep 8, 2018
* Ensure tests run minimally transpiled.

* Add specific test run for full transpilation.

* [BUGFIX beta] Prevent transpilation issues with @ember/ordered-set.

* Fix ES compatibility issues.

These cases were all previously working due to transpilation from `let`
to `var`. You *cannot* reference a block scoped variable (`let` /
`const`) during the variables own initializer.
@rwjblue rwjblue deleted the avoid-ordered-set-compilation-issue branch May 1, 2019 20:29
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