Skip to content

Support resourceQuery in context dependencies. - #5732

Merged
sokra merged 1 commit into
webpack:nextfrom
anuraaga:context-query
Nov 16, 2017
Merged

sokra merged 1 commit into
webpack:nextfrom
anuraaga:context-query

Conversation

@anuraaga

Copy link
Copy Markdown

What kind of change does this PR introduce?

Adds support for resource queries in context dependencies.

Did you add tests for your changes?

Yes

If relevant, link to documentation update:

N/A

Summary

Some loaders require resource query for proper usage, and cannot be used at all with context dependencies.

Does this PR introduce a breaking change?

No, it only affects code that adds a resource query to a context, which currently does not work at all.

Other information

Fixes #5730

@jsf-clabot

jsf-clabot commented Sep 28, 2017

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@sokra sokra left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good.

Comment thread lib/ContextModuleFactory.js Outdated
const dependencies = result.dependencies;

let loaders, resource, loadersPrefix = "";
let loaders, resource, loadersPrefix = "",

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

split this into multiple let declarations.

@sokra sokra left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@webpack-bot

Copy link
Copy Markdown
Contributor

@anuraaga Thanks for your update.

I labeled the Pull Request so reviewers will review it again.

@sokra Please review the new changes.

@sokra sokra closed this Oct 3, 2017
@sokra sokra reopened this Oct 3, 2017
@webpack-bot

Copy link
Copy Markdown
Contributor

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

@webpack-bot

Copy link
Copy Markdown
Contributor

The minimum test ratio has been reached. Thanks!

@anuraaga

Copy link
Copy Markdown
Author

@sokra Just wondering, when would this be merged into the next branch? It seems it needs a rebase, but would hold off if it'll just get out of date again

@anuraaga

anuraaga commented Nov 7, 2017

Copy link
Copy Markdown
Author

@sokra Ping?

@anuraaga

Copy link
Copy Markdown
Author

@sokra I have gone ahead and rebased this PR - do you think it can be merged? It would be depressing for it to get out of sync with the branch again...

@sokra
sokra merged commit 33c7970 into webpack:next Nov 16, 2017
@sokra

sokra commented Nov 16, 2017

Copy link
Copy Markdown
Member

Thanks

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.

4 participants