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 regression in Compilation.sortModules #7216

Merged
merged 3 commits into from May 7, 2018

Conversation

jordwest
Copy link

@jordwest jordwest commented May 7, 2018

What kind of change does this PR introduce?

Bugfix for regression introduced in #6203.

Did you add tests for your changes?

Yes

If relevant, link to documentation update:

N/A

Summary

This reverts the accidental change introduced in #6203 which sorted modules by id instead of index:

	sortModules(modules) {
-		modules.sort((a, b) => {
-			if(a.index < b.index) return -1;
-			if(a.index > b.index) return 1;
-			return 0;
-		});
+		modules.sort(byId);
	}

@sokra in #7194:

The change you found really looks incorrect. Looks like this was changed accidentally. Could you send a PR including a test case and change this back to index order.

the module.id is not available that this point, so that's wrong.

Does this PR introduce a breaking change?

No

Other information

This reverts the accidental change introduced
in webpack#6203 which sorted modules
by `id` instead of `index`.

See also: webpack#7194
@jsf-clabot
Copy link

jsf-clabot commented May 7, 2018

CLA assistant check
All committers have signed the CLA.

@webpack-bot
Copy link
Contributor

Thank you for your pull request! The most important CI builds succeeded, we’ll review the pull request soon.

Copy link
Member

@TheLarkInn TheLarkInn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great catch, and thank you for the contribution. 🙇

@sokra sokra merged commit 074cff8 into webpack:webpack-3 May 7, 2018
@sokra
Copy link
Member

sokra commented May 7, 2018

Thanks

@jordwest jordwest deleted the webpack-3 branch May 7, 2018 06:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants