Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Points out that outputFileSystem needs of methods #1265

Merged
merged 2 commits into from
Jun 3, 2017

Conversation

mc-zone
Copy link
Member

@mc-zone mc-zone commented Jun 1, 2017

the mkdirp and join are also needed by outputFileSystem.

@mc-zone
Copy link
Member Author

mc-zone commented Jun 1, 2017

OTOH, Should webpack auto make the polyfills if someone set an outputFileSystem without mkdirp or join?

@sokra have a miunte to give opinions?

the `mkdirp` and `join` are also needed by `outputFileSystem`.
@sokra
Copy link
Member

sokra commented Jun 1, 2017

OTOH, Should webpack auto make the polyfills if someone set an outputFileSystem without mkdirp or join?

no, they could be different for each fs.

@skipjack skipjack added the API label Jun 1, 2017
@mc-zone
Copy link
Member Author

mc-zone commented Jun 2, 2017

Guessing --root from input files: file:///home/travis/build/webpack/webpack.js.org/build/
should respond with HTTP status 200
404 http://cssnano.co/options/ at build/loaders/css-loader/index.html:507:182 ...

https://travis-ci.org/webpack/webpack.js.org/builds/238324866#L746

Can't pass travis CI check caused by this link is 404: http://cssnano.co/options/ .
It's not result from this PR. @skipjack Would you give some advice please?

@skipjack
Copy link
Collaborator

skipjack commented Jun 2, 2017

@mc-zone just opened a pull request (webpack-contrib/css-loader#550) to resolve this. css-loader's readme is dynamically pulled into this repo and the broken link was there.

Copy link
Collaborator

@skipjack skipjack left a comment

Choose a reason for hiding this comment

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

Minor wording change, aside from that this should be good to go (we don't have to wait on the css-loader pr).

@@ -277,4 +277,4 @@ compiler.run((err, stats) => {
});
```

T> The output file system you provide needs to be compatible with Node’s own [`fs`](https://nodejs.org/api/fs.html) module interface.
T> The output file system you provide needs to be compatible with Node’s own [`fs`](https://nodejs.org/api/fs.html) module interface, and other two helper methods `mkdirp`, `join` are also needed.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this would be better as:

module interface, which requires the `mkdirp` and `join` helper methods.

Copy link
Member Author

Choose a reason for hiding this comment

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

Right. Got it.

@skipjack skipjack merged commit c5eab88 into webpack:master Jun 3, 2017
@mc-zone mc-zone deleted the patch-3 branch June 4, 2017 03:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants