Skip to content
This repository was archived by the owner on Mar 17, 2021. It is now read-only.

feat: add context to options.outputPath && options.publicPath #166

Closed
wants to merge 3 commits into from

Conversation

coot
Copy link

@coot coot commented Jun 6, 2017

The context includes three fields userRequest, rawRequest and issuer context.

What kind of change does this PR introduce?
Adds functionality to outputPath and publicPath options (when set to functions)

Did you add tests for your changes?
Yes.

If relevant, did you update the README?
No, not yet, waiting for comments.

Summary
I am working on an react app in which I need to have relative path for files. The static assets are served from ./static folder. When the app is requesting images there are two cases:

  • they are requested from javascript bundle they need to have publicPath set to "./static/"
  • if they are requested from "style.css" which by itself is inside "./static" folder they need to be relative to "./" rather than "./static"

Does this PR introduce a breaking change?
No.

Other information

The context includes three fields userRequest, rawRequest and issuer context.
@jsf-clabot
Copy link

jsf-clabot commented Jun 6, 2017

CLA assistant check
All committers have signed the CLA.

@michael-ciniawsky michael-ciniawsky changed the title add context to config.outputPath and config.publicPath feat: add context to options.outputPath && options.publicPath Jun 6, 2017
@michael-ciniawsky michael-ciniawsky added this to the 0.12.0 milestone Jun 6, 2017
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.

Please add a test :)

index.js Outdated
@@ -50,9 +50,12 @@ module.exports = function(content) {
url = relativePath + url;
} else if (config.outputPath) {
// support functions as outputPath to generate them dynamically
var userRequest = this._module && this._module.userRequest
Copy link
Contributor

Choose a reason for hiding this comment

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

It would be neater to do

var module = this._module || {};
var userRequest = module.userRequest;

...

@joshwiens joshwiens modified the milestones: 1.1.0, 0.12.0 Jun 6, 2017
@joshwiens
Copy link
Member

@coot - I have set this to blocked ( #165 ) & moved this feature into 1.1.0, the next minor range after the pending 1.0.0 release.

@lsycxyj
Copy link

lsycxyj commented Jun 6, 2017

@coot Do you have any good idea to test this, especially things like this._module.issuer from Webpack API?

@coot
Copy link
Author

coot commented Jun 6, 2017

@michael-ciniawsky I added a simple test that checks that a second argument was passed to the publicPath and outputPath callbacks.
@lsycxyj I don't know how to test the issuer.

@alexander-akait
Copy link
Member

Close in favor #166. I think better pass this instead each property (more universally).

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.

7 participants