Skip to content
This repository has been archived by the owner on May 29, 2019. It is now read-only.

fix: refer to the entrypoint instead of the first module (module.identifier) #601

Merged
merged 1 commit into from
Oct 21, 2017

Conversation

akihikodaki
Copy link
Contributor

The order of modules can be reordered by optimize-module-order plugins. The intention is to use the entry point as the default identifier, so refer to entries property of the compilation.

@jsf-clabot
Copy link

jsf-clabot commented Aug 14, 2017

CLA assistant check
All committers have signed the CLA.

@codecov
Copy link

codecov bot commented Aug 14, 2017

Codecov Report

Merging #601 into master will increase coverage by 0.04%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #601      +/-   ##
==========================================
+ Coverage   87.41%   87.45%   +0.04%     
==========================================
  Files           7        7              
  Lines         302      303       +1     
  Branches       68       68              
==========================================
+ Hits          264      265       +1     
  Misses         36       36              
  Partials        2        2
Impacted Files Coverage Δ
src/loader.js 88.88% <100%> (+0.17%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5286ab2...f2c3056. Read the comment docs.

Copy link
Member

@alexander-akait alexander-akait left a comment

Choose a reason for hiding this comment

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

Can your describe more information (test repo would be best), also add tests for this, thanks!

@akihikodaki
Copy link
Contributor Author

Commit 426e758 adds a test case.

@akihikodaki
Copy link
Contributor Author

See also webpack/webpack.js.org#1527.

@akihikodaki
Copy link
Contributor Author

miraiken/web@46dfdde is an easy workaround.

@alexander-akait
Copy link
Member

@akihikodaki thanks for PR

@michael-ciniawsky michael-ciniawsky changed the title Refer to the entry point instead of the first module for default identifier refactor: refer to the entry point instead of the first module (module.identifier) Aug 16, 2017
@alexander-akait
Copy link
Member

@sokra need review

Copy link
Member

@sokra sokra left a comment

Choose a reason for hiding this comment

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

PR is good.

Seems like we missed updating ETP when webpack removed the entrypoint id = 0 limitation.

compilation.modules.forEach((module) => {
if (module.id === id) { item[0] = module.identifier(); }
if (typeof text === 'string') {
text = [[compilation.entries[0].identifier(), text]];
Copy link
Member

Choose a reason for hiding this comment

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

You can use [[entries[0].identifier(), text]] too i guess.

@alexander-akait
Copy link
Member

@akihikodaki friendly ping

@michael-ciniawsky
Copy link
Member

michael-ciniawsky commented Oct 19, 2017

Issues

@sgal
Copy link

sgal commented Oct 19, 2017

Also fixes webpack/webpack#5733

@akihikodaki
Copy link
Contributor Author

52a1942 applies #601 (review).

Copy link
Member

@michael-ciniawsky michael-ciniawsky left a comment

Choose a reason for hiding this comment

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

@joshwiens
Copy link
Member

@akihikodaki - Once #652 lands, this will need one more quick rebase before it merges.

@michael-ciniawsky
Copy link
Member

@akihikodaki #652 just landed please rebase when time :)

@akihikodaki
Copy link
Contributor Author

8228819: rebased to 5286ab2.

@michael-ciniawsky
Copy link
Member

Module build failed: TypeError: entries[0].identifier is not a function

@akihikodaki Can you revert to compilation.entries[0].identifier ? Or create a new variable etc ?

…tifier

The order of modules can be reordered by optimize-module-order plugins.
The intention is to use the entry point as the default identifier, so
refer to entries property of the compilation.
@akihikodaki
Copy link
Contributor Author

f2c3056: Reverted the change. I reviewed the intension why I used compilation.entries, and it was actually because entries argument is not an array of the first modules to be executed, but of chunks.

@michael-ciniawsky
Copy link
Member

michael-ciniawsky commented Oct 21, 2017

childCompiler.runAsChild((err, entries, compilation) => {
did you add this 🙃 ? The entries argument seems to be unused

@akihikodaki
Copy link
Contributor Author

No, I didn't.

@michael-ciniawsky michael-ciniawsky merged commit d5a1de2 into webpack-contrib:master Oct 21, 2017
@michael-ciniawsky michael-ciniawsky changed the title refactor: refer to the entry point instead of the first module (module.identifier) refactor: refer to the entrypoint instead of the first module (module.identifier) Oct 21, 2017
@michael-ciniawsky michael-ciniawsky changed the title refactor: refer to the entrypoint instead of the first module (module.identifier) fix: refer to the entrypoint instead of the first module (module.identifier) Oct 21, 2017
@cletusw
Copy link

cletusw commented Oct 21, 2017

Thanks! Could someone post here when we get a release bump with this fix?

@sgal
Copy link

sgal commented Oct 25, 2017

Is there any plan to release a new version with this fix? The issue with NamedModulesPlugin and HashedModuleIdsPlugin is preventing users from implementing long term caching, so it would be nice to get it out.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants