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

Configurable file system via options.fs #370

Merged
merged 3 commits into from
Feb 19, 2019
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ app.use(middleware(compiler, {
app.listen(3000, () => console.log('Example app listening on port 3000!'))
```


hinell marked this conversation as resolved.
Show resolved Hide resolved
## Options

The middleware accepts an `options` Object. The following is a property reference
Expand Down Expand Up @@ -210,7 +211,7 @@ please see the [webpack documentation](https://webpack.js.org/configuration/watc
Type: `Boolean|Function`
Default: `false`

If true, the option will instruct the module to write files to the configured
If `true`, the option will instruct the module to write files to the configured
location on disk as specified in your `webpack` config file. _Setting
`writeToDisk: true` won't change the behavior of the `webpack-dev-middleware`,
and bundle files accessed through the browser will still be served from memory._
Expand All @@ -231,6 +232,14 @@ of `true` _will_ write the file to disk. eg.
}
```

### fs
Type: `Object`
Default: `MemoryFileSystem`

Set the default file system which will be used by webpack as primary destination of generated files. Default is set to webpack's default file system: [memory-fs](https://github.com/webpack/memory-fs). This option isn't affected by the [writeToDisk](#writeToDisk) option.

**Note:** As of 3.5.x version of the middleware you have to provide `.join()` method to the `fs` instance. This can be done simply by using `path.join`.

hinell marked this conversation as resolved.
Show resolved Hide resolved
## API

`webpack-dev-middleware` also provides convenience methods that can be use to
Expand Down
16 changes: 14 additions & 2 deletions lib/fs.js
Original file line number Diff line number Diff line change
Expand Up @@ -60,9 +60,21 @@ module.exports = {

let fileSystem;
// store our files in memory
const isMemoryFs = !compiler.compilers && compiler.outputFileSystem instanceof MemoryFileSystem;
const isConfiguredFs = context.options.fs;
const isMemoryFs = !isConfiguredFs
&& !compiler.compilers
&& compiler.outputFileSystem instanceof MemoryFileSystem;

if (isMemoryFs) {
if (isConfiguredFs) {
const { fs } = context.options;
if (typeof fs.join !== 'function') {
Copy link
Member

Choose a reason for hiding this comment

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

Just questions, can you find place where we use fs.join? Looks like we should remove this/deprecated from code base.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is not used by the webpack-middleware but rather by wepback instead. Here it is as a precaution to avoid digging up webpack's erros. If you attempt to provide fs without join() webpack will complain about it when accessing webpack.outputFileSystem.

I can switch to Russian to explain more if you like.

Copy link
Member

@alexander-akait alexander-akait Feb 18, 2019

Choose a reason for hiding this comment

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

I can switch to Russian to explain more if you like.

No need

It is not used by the webpack-middleware but rather by wepback instead. Here it is as a precaution to avoid digging up webpack's erros. If you attempt to provide fs without join() webpack will complain about it when accessing webpack.outputFileSystem.

I think we should open issue in webpack and remove all fs.join in webpack@5, join is not part of fs and looks very dirty and misleading

Can you open issue?

Copy link
Contributor Author

@hinell hinell Feb 18, 2019

Choose a reason for hiding this comment

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

Well I got no time for that, sorry. May be next time.

Copy link
Member

Choose a reason for hiding this comment

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

@hinell i mean just create issue in webpack repo and all, i am from mobile and it is hard for me right now

// very shallow check
throw new Error('Invalid options: options.fs.join() method is expected');
}

compiler.outputFileSystem = fs;
fileSystem = fs;
} else if (isMemoryFs) {
fileSystem = compiler.outputFileSystem;
} else {
fileSystem = new MemoryFileSystem();
Expand Down
34 changes: 34 additions & 0 deletions test/tests/file-system.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,40 @@ describe('FileSystem', () => {
assert.equal(firstFs, secondFs);
});

describe('options.fs', () => {
// lightweight compiler mock
const hook = { tap() {} };
const compiler = {
outputPath: '/output',
watch() {},
hooks: { done: hook, invalid: hook, run: hook, watchRun: hook }
};

const fs = { join() {} };

it('should throw on invalid fs', (done) => {
assert.throws(() => {
middleware(compiler, { fs: {} });
});
done();
});

it('should assign fs to the compiler.outputFileSystem', (done) => {
const instance = middleware(compiler, { fs });

assert.equal(compiler.outputFileSystem, fs);
instance.close(done);
});
it('should go safely when compiler.outputFileSystem is assigned by fs externally', (done) => {
hinell marked this conversation as resolved.
Show resolved Hide resolved
const cmplr = Object.create(compiler);
cmplr.outputFileSystem = fs;
const instance = middleware(cmplr, { fs });

assert.equal(cmplr.outputFileSystem, fs);
instance.close(done);
});
});

it('should throw on invalid outputPath config', () => {
const compiler = fakeWebpack();
compiler.outputPath = './dist';
Expand Down