use memfs instead of memory-fs #366

heygrady · 2019-02-09T06:45:11Z

Uses memfs instead of memory-fs for the in-memory outputFileSystem. memfs appears to be actively maintained, while memory-fs hasn't had any meaningful updates in a few years.

This allows people to use out-of-the-box webpack-dev-server for novel use-cases where having a fully API-compliant filesystem would be beneficial. Related: webpack/memory-fs#43

I ran into issues adding my own server-side-rendering middleware into webpack-dev-server that needed to require files from the outputFileSystem. Doing this with memory-fs was difficult, time consuming and seemed not to work very reliably. My alternative was to write to disk, which means 30 second re-builds.

Interestingly, memfs is part of a suite of tools that enables useful stuff like patching require to import modules from an in-memory file system.

This was a simple drop-in replacement.

swap memory-fs with memfs
move the join and normalize methods from memory-fs to "a better place"
All tests pass

jsf-clabot · 2019-02-09T06:45:12Z

All committers have signed the CLA.

codecov · 2019-02-09T06:49:51Z

Codecov Report

Merging #366 into master will increase coverage by 0.02%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #366      +/-   ##
==========================================
+ Coverage   96.85%   96.87%   +0.02%     
==========================================
  Files           7        7              
  Lines         254      256       +2     
==========================================
+ Hits          246      248       +2     
  Misses          8        8

Impacted Files	Coverage Δ
lib/fs.js	`95.45% <100%> (+0.21%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8e54af2...89524ae. Read the comment docs.

codecov · 2019-02-09T06:49:51Z

Codecov Report

Merging #366 into master will increase coverage by 0.04%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #366      +/-   ##
==========================================
+ Coverage   96.85%   96.89%   +0.04%     
==========================================
  Files           7        7              
  Lines         254      258       +4     
==========================================
+ Hits          246      250       +4     
  Misses          8        8

Impacted Files	Coverage Δ
lib/fs.js	`95.65% <100%> (+0.41%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8e54af2...fbc4932. Read the comment docs.

alexander-akait · 2019-02-09T14:34:55Z

We need fix it webpack first https://github.com/webpack/webpack/blob/b934e26da8b65bdf36b3c71141d0c2d8397d9f44/package.json#L22

https://github.com/webpack/webpack/blob/8a7597aa6eb2eef66a8f9db3a0c49bcb96022a94/lib/MemoryOutputFileSystem.js

alexander-akait · 2019-02-09T14:37:51Z

lib/fs.js

+const { fs: memfs } = require('memfs');
+// borrow join and normalize from memory-fs
+memfs.join = require('memory-fs/lib/join');
+memfs.normalize = require('memory-fs/lib/normalize');


It is not good solution, we don't need two memory-fs package

Those functions appear to be unrelated to the file system itself and could live somewhere else.

I know, but it is bad have two memory fs package, we should move these function to separate repo or move to memfs

alexander-akait · 2019-02-09T14:39:57Z

Also can you provide example where

I ran into issues adding my own server-side-rendering middleware into webpack-dev-server that needed to require files from the outputFileSystem. Doing this with memory-fs was difficult, time consuming and seemed not to work very reliably. My alternative was to write to disk, which means 30 second re-builds.

heygrady · 2019-02-09T17:03:11Z

Original comment moved here: https://gist.github.com/heygrady/ae6ec0d725c6c68974802ba5d8328617

difficult, time consuming and seemed not to work very reliably.

This is what I meant:

In order to import a file from memory-fs you need to use require-from-string
When you use require-from-string you can't use code-splitting
When you use require-from-string you can't use node-externals (sometimes)
When you read from a buffer, everything becomes async
Using fs-monkey with memfs and unionfs would side-step all of those problems.

// importFromMemoryFs.js
const { promisify } = require('util')
const requireFromString = require('require-from-string')

let cache = {}

const importFromMemoryFs = async (stats, pathname) => {
  const { compilation, hash } = stats
  const { outputFileSystem } = compilation.compiler
  if (cache[hash] && cache[hash][pathname]) {
    return cache[hash][pathname]
  } else if (!cache[hash]) {
    cache = { [hash]: {} }
  }
  const readFileAsync = promisify(outputFileSystem.readFile.bind(outputFileSystem))
  try {
    const buffer = await readFileAsync(pathname, 'utf8')
    const source = buffer.toString()
    const module = requireFromString(source, pathname)
    cache[hash][pathname] = module
    return module
  } catch (error) {
    console.error(error)
  }
}

module.exports = importFromMemoryFs

In my project I'm using webpack-dev-server with server-side-rendering. I was able to get it all working roughly as described here.

// ssr-dev-middleware.js
const path = require('path')
const { wrap } = require('async-middleware')

const importFromMemoryFs = './importFromMemoryFs'

const pathToMain = path.join(__dirname, '..', 'dist', 'server', 'main.js')

module.exports = wrap(async (req, res, next) => {
  const stats = res.locals.webpackStats.stats[1]
  const middleware = await importFromMemoryFs(stats, pathToMain)
  middleware(req, res, () => undefined)
})

heygrady · 2019-02-09T19:30:31Z

streamich/memfs#325 (comment)

I created a fork of memory-fs and tried to replace it with Volume. It's mostly working but some tests are failing. Most of the test failures are related to streams.

There are also some failures related to windows-style paths. I think those might be due to memory-fs using a normalize function on all paths and I didn't wrap the memfs methods to do that.

I'm exploring easy ways to make memory-fs API-compliant to work with webpack-dev-server.

https://github.com/heygrady/memory-fs/tree/memfs

Update

webpack/memory-fs#66

I was able to get every test passing except for the tests with win32 paths starting with C:\. It appears that memfs doesn't handle those.

Some differences:

memfs streams behave like fs does. This causes some tests to fail where memory-fs streams were non-standard.
memfs has a concept of "relative" paths that uses process.cwd (similar to fs). This causes some tests to fail where memory-fs normally throws on relative paths.
memfs does not handle win32 paths starting with C:\ (when you are on a posix machine).

Switching to memfs would probably be blocked by the difference in win32 support. I might've overlooked something obvious but the memory-fs tests with paths starting with C:\ totally failed with memfs.

hinell · 2019-02-18T17:30:16Z

This is nice PR but I got far better solution I believe. #370

alexander-akait · 2019-02-18T17:53:36Z

@hinell yep, #370 better

If somebody have problem with memory-fs please open issue

laynef · 2019-06-28T20:19:11Z

@hinell yep, #370 better

If somebody have problem with memory-fs please open issue

I am currently having issues with maxing out memory, I've written my files to disk and that is not helping

hinell · 2019-06-28T21:53:08Z

@laynef It's better to open a separate issue on that. It would be also nice to elaborate it with details. So far everything it worked smoothly for me.

alexander-akait · 2019-08-29T11:16:15Z

You can configure fs using options, maybe we revisit default memory fs in future, no need right now

alexander-akait · 2019-08-29T11:16:27Z

Anyway thank you for the PR

heygrady requested a review from alexander-akait as a code owner February 9, 2019 06:45

heygrady requested a review from michael-ciniawsky as a code owner February 9, 2019 06:45

use memfs instead of memory-fs

b6853ac

heygrady force-pushed the memfs branch from 89524ae to b6853ac Compare February 9, 2019 06:57

alexander-akait reviewed Feb 9, 2019

View reviewed changes

Use Volume; revert test

fbc4932

heygrady mentioned this pull request Feb 9, 2019

creates undefined directories streamich/memfs#325

Closed

This was referenced Feb 10, 2019

Support win32 paths streamich/memfs#327

Open

Trying to find async import() files streamich/memfs#323

Closed

Use memfs under the hood webpack/memory-fs#66

Open

alexander-akait closed this Aug 29, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

use memfs instead of memory-fs #366

use memfs instead of memory-fs #366

heygrady commented Feb 9, 2019 •

edited

Loading

jsf-clabot commented Feb 9, 2019 •

edited

Loading

codecov bot commented Feb 9, 2019

codecov bot commented Feb 9, 2019 •

edited

Loading

alexander-akait commented Feb 9, 2019 •

edited

Loading

alexander-akait Feb 9, 2019

heygrady Feb 9, 2019

alexander-akait Feb 9, 2019

alexander-akait commented Feb 9, 2019

heygrady commented Feb 9, 2019 •

edited

Loading

heygrady commented Feb 9, 2019 •

edited

Loading

hinell commented Feb 18, 2019 •

edited

Loading

alexander-akait commented Feb 18, 2019

laynef commented Jun 28, 2019

hinell commented Jun 28, 2019

alexander-akait commented Aug 29, 2019

alexander-akait commented Aug 29, 2019

use memfs instead of memory-fs #366

use memfs instead of memory-fs #366

Conversation

heygrady commented Feb 9, 2019 • edited Loading

jsf-clabot commented Feb 9, 2019 • edited Loading

codecov bot commented Feb 9, 2019

Codecov Report

codecov bot commented Feb 9, 2019 • edited Loading

Codecov Report

alexander-akait commented Feb 9, 2019 • edited Loading

alexander-akait Feb 9, 2019

Choose a reason for hiding this comment

heygrady Feb 9, 2019

Choose a reason for hiding this comment

alexander-akait Feb 9, 2019

Choose a reason for hiding this comment

alexander-akait commented Feb 9, 2019

heygrady commented Feb 9, 2019 • edited Loading

heygrady commented Feb 9, 2019 • edited Loading

Update

hinell commented Feb 18, 2019 • edited Loading

alexander-akait commented Feb 18, 2019

laynef commented Jun 28, 2019

hinell commented Jun 28, 2019

alexander-akait commented Aug 29, 2019

alexander-akait commented Aug 29, 2019

heygrady commented Feb 9, 2019 •

edited

Loading

jsf-clabot commented Feb 9, 2019 •

edited

Loading

codecov bot commented Feb 9, 2019 •

edited

Loading

alexander-akait commented Feb 9, 2019 •

edited

Loading

heygrady commented Feb 9, 2019 •

edited

Loading

heygrady commented Feb 9, 2019 •

edited

Loading

hinell commented Feb 18, 2019 •

edited

Loading