Fail to load named modules when using ConstArray dependencies#5771
Fail to load named modules when using ConstArray dependencies#5771sokra merged 2 commits intowebpack:masterfrom
Conversation
|
Thank you for your pull request! The most important CI builds succeeded, we’ll review the pull request soon. |
|
|
||
| define(["named1", "named2"], function(named1, named2) { | ||
| it("should load the named modules in defined dependencies", function() { | ||
| "named1".should.be.eql(named1); |
There was a problem hiding this comment.
Could you switch variable and string so the expected and actual values are correct?
named1.should.be.eql("named1");There was a problem hiding this comment.
OK. The only reason I did it that way was to avoid throwing an exception if the variable named1 is undefined (e.g. "Unable to read property should of undefined").
There was a problem hiding this comment.
BTW, could you provide pointers to webpack-bot info? I like the review workflow that you've implemented and would like to do something similar with projects I work on. Thanks.
There was a problem hiding this comment.
|
@chuckdumont Thanks for your update. I labeled the Pull Request so reviewers will review it again. @sokra Please review the new changes. |
What kind of change does this PR introduce? Bug fix
Did you add tests for your changes? Yes
If relevant, link to documentation update: N/A
Summary Fix failure to load named modules (AMD) when using constArray dependencies.
Does this PR introduce a breaking change? No
Other information Wasn't sure about the location of the unit tests. Let me know if you want them moved.