Skip to content

Conversation

jpommerening
Copy link
Contributor

@jpommerening jpommerening commented Nov 15, 2016

Hi!

First: Thanks for releasing this wonderful loader! This pull request allows for the publicPath to be set via query parameter.

Let me try to explain why I need this.

My project's directory structure looks like this:

/dist                            (=> publicPath)
/dist/main.js                    (=> bundled webpack output)
/dist/assets/f00b43-main.css     (=> output of 'file-loader!extract-loader!css-loader')
/dist/assets/f001ce-img.png      (=> output of 'file-loader!img-loader', ref'd by main.css)
/index.html                      (=> browser entry point)

(other paths are just sources for webpack)

I'm using a relative publicPath in my webpack config, so that the bundled main.js and extracted CSS don't depend on the path where I deploy my project:

module.exports = {
  output: {
    path: path.resolve( 'dist/' ),
    publicPath: 'dist/', // no leading slash!
    filename: '[name].js'
  },
  /* ... */

I've set up my file-loader to put files into the assets subdirectory inside the output path:

  /* ... */
  fileLoader: {
    name: 'assets/[name]-[sha1:hash:hex:6].[ext]'
  }
};

Bundling and hot reload (sometimes finicky to get right in conjunction with publicPath) both work as expected with this setup.

However, issues arise when the extracted and file-loader'ed CSS file has references to other files, images for example. Because of my relative publicPath the extract-loader generates a CSS file that contains url(dist/assets/f001ce-img.png) just like you would expect. But since the file-loader puts the extracted CSS into a file that is already in that same directory, that URL now point at dist/assets/dist/assets/f001ce-img.png. (Note: the problem would even arise if I'd omit the sub directory in my file loader config.)

To alleviate that issue I want to be able to set the public path manually when extracting and passing the output to the file-loader. In my case, I'd just add ?publicPath=../ to the extract-loader to step back up into dist/ from dist/assets/.

Maybe I'm taking things too far with my setup, but the change is pretty minor and backwards compatible. Also, the loader-wrapper-thing in extract-text-webpack-plugin has the same option. :)

@coveralls
Copy link

coveralls commented Nov 15, 2016

Coverage Status

Coverage increased (+0.1%) to 98.039% when pulling 906719a on jpommerening:feature-add-publicPath-option into 75ed1fa on peerigon:master.

@jpommerening
Copy link
Contributor Author

Ping @jhnns :)

@jhnns
Copy link
Member

jhnns commented Nov 17, 2016

That's a great change! 👍
It's also better because webpack 2 discourages reading from this.options.

Could you add that option to the README (under options :))?

@jpommerening
Copy link
Contributor Author

@coveralls
Copy link

Coverage Status

Coverage increased (+0.1%) to 98.039% when pulling 3d0cf2b on jpommerening:feature-add-publicPath-option into 75ed1fa on peerigon:master.

1 similar comment
@coveralls
Copy link

coveralls commented Nov 17, 2016

Coverage Status

Coverage increased (+0.1%) to 98.039% when pulling 3d0cf2b on jpommerening:feature-add-publicPath-option into 75ed1fa on peerigon:master.

@coveralls
Copy link

coveralls commented Nov 17, 2016

Coverage Status

Coverage increased (+0.1%) to 98.039% when pulling 3d0cf2b on jpommerening:feature-add-publicPath-option into 75ed1fa on peerigon:master.

@jhnns jhnns merged commit 8a1824f into peerigon:master Nov 25, 2016
@jhnns
Copy link
Member

jhnns commented Nov 25, 2016

Awesome, thx. Shipped as 0.1.0

@jpommerening
Copy link
Contributor Author

Thanks! 🎉

@jpommerening jpommerening deleted the feature-add-publicPath-option branch November 25, 2016 12:53
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.

3 participants