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.

Sign up for GitHub

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

Instantiate MemoryFileSystem only once #120

Merged
merged 4 commits into from
Sep 13, 2016
Merged

Instantiate MemoryFileSystem only once #120

Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f49f1d5
Add option to specify custom compiler fileSystem; Only create MemoryF…
moljac024 Sep 8, 2016
170aa08
Add comment about additional option for reusing in-memory fs
moljac024 Sep 8, 2016
926fabb
Fix beautify-lint errors
moljac024 Sep 8, 2016
c6f804e
Remove options.fileSystem
moljac024 Sep 10, 2016
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
14 changes: 13 additions & 1 deletion middleware.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,19 @@ module.exports = function(compiler, options) {
if(typeof options.reporter !== "function") options.reporter = defaultReporter;

// store our files in memory
var fs = compiler.outputFileSystem = new MemoryFileSystem();
var fs;
if(options.fileSystem) {
Copy link
Member

Choose a reason for hiding this comment

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

Does your usecase still work when you remove the options.fileSystem? It looks now as if you've implemented two ways of reusing the outputFileSystem; by allowing options.fileSystem AND by checking if compiler.outputFileSystem already exists.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, my usecase will work with the removal of options.fileSystem. I'm not even using it currently but I added it here because it might be a better solution than checking the outputFileSystem.

Or maybe we want both for flexibility. Like I said, I'm willing to make any changes necessary, so long as there is an option for not mutating the outputFileSystem.

Copy link
Member

Choose a reason for hiding this comment

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

Okay, could you remove options.fileSystem then? After that I'll do some tests and merge this in.

Copy link
Contributor Author

@moljac024 moljac024 Sep 10, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

Done.

EDIT: I have also changed the pull request title, removing the part that referenced the deleted code.

fs = compiler.outputFileSystem = options.fileSystem;
} else {
// TODO: Probably have a config for this as well, e.g. something like:
// options.reuseMemoryFs
var isMemoryFs = compiler.outputFileSystem instanceof MemoryFileSystem;
if(isMemoryFs) {
fs = compiler.outputFileSystem;
} else {
fs = compiler.outputFileSystem = new MemoryFileSystem();
}
}

compiler.plugin("done", function(stats) {
// We are now on valid state
Expand Down