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

Fix system transformer to ensure consistent modules iteration #4832

Merged
merged 2 commits into from
Nov 14, 2016

Conversation

guybedford
Copy link
Contributor

Q A
Bug fix? yes
Breaking change? no
New feature? no
Deprecations? no
Spec compliancy? n/a
Tests added/pass? yes
License MIT

Currently in the system module transformer, the list of module specifiers modules is used as an ordered object and iterated via for (let sourceName in modules) to output the dependency and setters lists. Relying on plain object property ordering doesn't work for numeric values in JavaScript though.

To make this process more consistent this updates modules to an array which can then be iterated reliably. It fixes faulty ordering for cases where module names are numbers, as in the test provided.

@hzoo hzoo added the PR: Bug Fix 🐛 A type of pull request used for our changelog categories label Nov 11, 2016
@codecov-io
Copy link

codecov-io commented Nov 11, 2016

Current coverage is 89.28% (diff: 100%)

Merging #4832 into master will increase coverage by <.01%

@@             master      #4832   diff @@
==========================================
  Files           196        196          
  Lines         13812      13816     +4   
  Methods        1434       1436     +2   
  Messages          0          0          
  Branches       3210       3211     +1   
==========================================
+ Hits          12331      12335     +4   
  Misses         1481       1481          
  Partials          0          0          

Powered by Codecov. Last update 5678e61...76702a3

guybedford added a commit to systemjs/builder that referenced this pull request Nov 12, 2016
@hzoo hzoo merged commit 62ae3c7 into babel:master Nov 14, 2016
guybedford added a commit to systemjs/builder that referenced this pull request Jan 19, 2017
guybedford added a commit to systemjs/builder that referenced this pull request Jan 23, 2017
guybedford added a commit to systemjs/builder that referenced this pull request Jan 23, 2017
guybedford added a commit to systemjs/builder that referenced this pull request Jan 24, 2017
@lock lock bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Oct 7, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Oct 7, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
outdated A closed issue/PR that is archived due to age. Recommended to make a new issue PR: Bug Fix 🐛 A type of pull request used for our changelog categories
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants