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

Compiler.outputFileSystem.constructor of Webpack can be undefined in webpack 5.0 #384

Closed
wants to merge 3 commits into from

Conversation

iamssen
Copy link

@iamssen iamssen commented Oct 12, 2020

image

After webpack 5.0 update BundleAnalyzerPlugin.getBundleDirFromCompiler() throws errors.

Compiler.outputFileSystem.constructor is undefined.

After this fix, my configuration works fine in the webpack 5.0.

@jsf-clabot
Copy link

jsf-clabot commented Oct 12, 2020

CLA assistant check
All committers have signed the CLA.

@iamssen iamssen mentioned this pull request Oct 12, 2020
2 tasks
return null;
default:
return this.compiler.outputPath;
if (this.compiler.outputFileSystem.constructor) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

We need to understand why it's not available now and how to get FS type in webpack 5. @sokra?

Copy link
Member

Choose a reason for hiding this comment

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

@th0r we drop memory-fs in favor memfs package

@iamssen
Copy link
Author

iamssen commented Nov 19, 2020

I don't know exactly.

In webpack 4.x, outputFileSystem is set to class NodeOutputFileSystem. https://github.com/webpack/webpack/blob/webpack-4/lib/node/NodeEnvironmentPlugin.js#L35

However, in webpack 5.x, outputFileSystem is set to graceful-fs. https://github.com/webpack/webpack/blob/master/lib/node/NodeEnvironmentPlugin.js#L39

As I checked, after webpack 5.x, it seems that outputFileSystem should be an fs implementation, not a class instance.

I couldn't find a way to check if the outputFileSystem of webpack 5.x is graceful-fs (default) or memfs.

See more:

@valscion
Copy link
Member

Can we have some sort of spec that he code works for all versions of webpack we have?

You can see an example of testing different kinds of webpack configurations here:

forEachWebpackVersion(({it, webpackCompile}) => {
it('should allow to generate json report', async function () {

Remember to check that a new test case would break without the fix here.

}
// TODO Can we check the outputFileSystem is graceful-fs or memfs on webpack 5.x?
// If avaliable, we can return null if the outputFileSystem is memfs.
return this.compiler.outputPath;
Copy link
Collaborator

Choose a reason for hiding this comment

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

The thing is in webpack 5 it will return this.compiler.outputPath even in dev mode (when webpack-dev-server is used) and will try to parse compiled bundle which is not there.
Seems like we need some other way to detect non-physical file systems now.

@ludofischer
Copy link

ludofischer commented Dec 15, 2020

I cannot reproduce a crash with the latest webpack-dev-server beta.

  • I do not get crashes when using bundle-analyzer with the latest webpack-dev-middleware@4.0.2.
  • I do not get crashes using webpack-dev-server@4.0.0-beta.0 which uses the lates webpack-dev-middleware

My setup: https://github.com/ludofischer/bundle-analyzer-webpack5

@iamssen Does the error occur when using webpack-dev-server? In your pull request https://github.com/rocket-hangar/rocket-scripts/pull/25/files, the webpack-dev-server is still at ^3.10 which depends on webpack 4, so I don't understand how webpack 5 gets called (my mistake, it's a peer dependency on webpack ^4.0.0 || ^5.0.0). You could try updating to webpack-dev-server@4.0.0-beta.0 and see what happens.

@iamssen iamssen closed this Apr 22, 2021
@kedarv
Copy link
Contributor

kedarv commented May 7, 2021

I can reproduce this error in Jest. Here's a minimal repro:

bundle.test.js

 import tmp from 'tmp';
 import webpack from 'webpack';
 import BundleAnalyzerPlugin from 'webpack-bundle-analyzer';
 
 function runWebpack(cb) {
    tmp.dir((err, path, cleanupCallback) => {
        const compiler = webpack({
            entry: 'noop-entry.js',
            output: { path },
            plugins: [new BundleAnalyzerPlugin.BundleAnalyzerPlugin()],
        });
        compiler.run(cb);
        cleanupCallback();
    });
 }
 
 describe('some test suite', () => {
    beforeEach(done => {
        runWebpack(err => {
            console.log(err);
            done();
        });
    });

    it('some test', () => {
       console.log("assert something..")
    });
});

(noop-entry.js is just an empty file)

package.json

{
  "name": "test-webpack",
  "version": "1.0.0",
  "main": "index.js",
  "license": "MIT",
  "dependencies": {
    "tmp": "^0.2.1",
    "webpack": "^5.36.2",
    "webpack-bundle-analyzer": "^4.4.1"
  },
  "type": "module",
  "devDependencies": {
    "jest": "^26.6.3"
  },
  "scripts": {
    "test": "jest"
  }  
}

NODE_OPTIONS=--experimental-vm-modules yarn jest results in:

  console.log
    TypeError: Cannot read property 'name' of undefined
        at BundleAnalyzerPlugin.getBundleDirFromCompiler (/Users/kedar/webpack-bundle-analyzer-repro/node_modules/webpack-bundle-analyzer/lib/BundleAnalyzerPlugin.js:144:56)
        at BundleAnalyzerPlugin.startAnalyzerServer (/Users/kedar/webpack-bundle-analyzer-repro/node_modules/webpack-bundle-analyzer/lib/BundleAnalyzerPlugin.js:114:25)
        at /Users/kedar/webpack-bundle-analyzer-repro/node_modules/webpack-bundle-analyzer/lib/BundleAnalyzerPlugin.js:62:33
        at /Users/kedar/webpack-bundle-analyzer-repro/node_modules/webpack-bundle-analyzer/lib/BundleAnalyzerPlugin.js:73:53
        at Array.map (<anonymous>)
        at Immediate.<anonymous> (/Users/kedar/webpack-bundle-analyzer-repro/node_modules/webpack-bundle-analyzer/lib/BundleAnalyzerPlugin.js:73:39)
        at processImmediate (node:internal/timers:464:21)

      at bundle.test.js:20:21

It's unclear why this error is happening right now -- I will note though that this test would pass (ie. Webpack Bundle Analyzer starts up and serves data) under Webpack 4 and the v3 branch of this library.

@alexander-akait @th0r let me know if you'd like me to file an issue instead of adding stuff onto this closed PR.

PS: the tmp part is unnecessary to repro, I just added it here since it's representative of a real test.

@valscion
Copy link
Member

Can you open a new pull request with the failing test so we can see the failure in this repos CI, too? :)

@kedarv
Copy link
Contributor

kedarv commented May 10, 2021

Can you open a new pull request with the failing test so we can see the failure in this repos CI, too? :)

I'm not able to reproduce in this repo itself :/ (this.compiler.outputFileSystem.constructor.name is defined)
I wonder if there's some interaction between webpack and jest that I'm not aware of.

I added a print in NodeEnvironmentPlugin and observed the constructor property is undefined there too.

@alexander-akait
Copy link
Member

Somebody change this.compiler.outputFileSystem, you can add proxy and look who touch this https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Proxy

@kedarv
Copy link
Contributor

kedarv commented May 10, 2021

@alexander-akait: I added a console log statement in NodeEnvironmentPlugin where outputFileSystem is set:

		compiler.outputFileSystem = fs;
		console.log(compiler.outputFileSystem.constructor)

And here constructor is already undefined. So I'm not sure if it's being mutated or if this is just expected? Does graceful-fs even have this property 🤔

@alexander-akait
Copy link
Member

alexander-akait commented May 10, 2021

Can you show console.log(compiler.outputFileSystem) output? Maybe also console.log(compiler.outputFileSystem.toString())

and run npm ls webpack/yarn why webpack

@kedarv
Copy link
Contributor

kedarv commented May 10, 2021

  console.log
    [Object: null prototype] {
      appendFile: [Function: appendFile],
      appendFileSync: [Function: appendFileSync],
      access: [Function: access],
      accessSync: [Function: accessSync],
      chown: [Function (anonymous)],
      chownSync: [Function (anonymous)],
      chmod: [Function (anonymous)],
      chmodSync: [Function (anonymous)],
      close: [Function: close],
      closeSync: [Function: closeSync],
      copyFile: [Function: copyFile],
      copyFileSync: [Function: copyFileSync],
      createReadStream: [Function: createReadStream],
      createWriteStream: [Function: createWriteStream],
      exists: [Function: exists],
      existsSync: [Function: existsSync],
      fchown: [Function (anonymous)],
      fchownSync: [Function (anonymous)],
      fchmod: [Function (anonymous)],
      fchmodSync: [Function (anonymous)],
      fdatasync: [Function: fdatasync],
      fdatasyncSync: [Function: fdatasyncSync],
      fstat: [Function (anonymous)],
      fstatSync: [Function (anonymous)],
      fsync: [Function: fsync],
      fsyncSync: [Function: fsyncSync],
      ftruncate: [Function: ftruncate],
      ftruncateSync: [Function: ftruncateSync],
      futimes: [Function: futimes],
      futimesSync: [Function: futimesSync],
      lchown: [Function (anonymous)],
      lchownSync: [Function (anonymous)],
      lchmod: [Function (anonymous)],
      lchmodSync: [Function (anonymous)],
      link: [Function: link],
      linkSync: [Function: linkSync],
      lstat: [Function (anonymous)],
      lstatSync: [Function (anonymous)],
      lutimes: [Function: lutimes],
      lutimesSync: [Function: lutimesSync],
      mkdir: [Function: mkdir],
      mkdirSync: [Function: mkdirSync],
      mkdtemp: [Function: mkdtemp],
      mkdtempSync: [Function: mkdtempSync],
      open: [Function: open],
      openSync: [Function: openSync],
      opendir: [Function: opendir],
      opendirSync: [Function: opendirSync],
      readdir: [Function: readdir],
      readdirSync: [Function: readdirSync],
      read: [Function: read],
      readSync: [Function (anonymous)],
      readv: [Function: readv],
      readvSync: [Function: readvSync],
      readFile: [Function: readFile],
      readFileSync: [Function: readFileSync],
      readlink: [Function: readlink],
      readlinkSync: [Function: readlinkSync],
      realpath: [Function: realpath] { native: [Function (anonymous)] },
      realpathSync: [Function: realpathSync] { native: [Function (anonymous)] },
      rename: [Function: rename],
      renameSync: [Function: renameSync],
      rm: [Function: rm],
      rmSync: [Function: rmSync],
      rmdir: [Function: rmdir],
      rmdirSync: [Function: rmdirSync],
      stat: [Function (anonymous)],
      statSync: [Function (anonymous)],
      symlink: [Function: symlink],
      symlinkSync: [Function: symlinkSync],
      truncate: [Function: truncate],
      truncateSync: [Function: truncateSync],
      unwatchFile: [Function: unwatchFile],
      unlink: [Function: unlink],
      unlinkSync: [Function: unlinkSync],
      utimes: [Function: utimes],
      utimesSync: [Function: utimesSync],
      watch: [Function: watch],
      watchFile: [Function: watchFile],
      writeFile: [Function: writeFile],
      writeFileSync: [Function: writeFileSync],
      write: [Function: write],
      writeSync: [Function: writeSync],
      writev: [Function: writev],
      writevSync: [Function: writevSync],
      Dir: [class Dir],
      Dirent: [class Dirent],
      Stats: [Function: Stats],
      ReadStream: [Getter/Setter],
      WriteStream: [Getter/Setter],
      FileReadStream: [Getter/Setter],
      FileWriteStream: [Getter/Setter],
      _toUnixTimestamp: [Function: toUnixTimestamp],
      F_OK: 0,
      R_OK: 4,
      W_OK: 2,
      X_OK: 1,
      constants: [Object: null prototype] {
        UV_FS_SYMLINK_DIR: 1,
        UV_FS_SYMLINK_JUNCTION: 2,
        O_RDONLY: 0,
        O_WRONLY: 1,
        O_RDWR: 2,
        UV_DIRENT_UNKNOWN: 0,
        UV_DIRENT_FILE: 1,
        UV_DIRENT_DIR: 2,
        UV_DIRENT_LINK: 3,
        UV_DIRENT_FIFO: 4,
        UV_DIRENT_SOCKET: 5,
        UV_DIRENT_CHAR: 6,
        UV_DIRENT_BLOCK: 7,
        S_IFMT: 61440,
        S_IFREG: 32768,
        S_IFDIR: 16384,
        S_IFCHR: 8192,
        S_IFBLK: 24576,
        S_IFIFO: 4096,
        S_IFLNK: 40960,
        S_IFSOCK: 49152,
        O_CREAT: 512,
        O_EXCL: 2048,
        UV_FS_O_FILEMAP: 0,
        O_NOCTTY: 131072,
        O_TRUNC: 1024,
        O_APPEND: 8,
        O_DIRECTORY: 1048576,
        O_NOFOLLOW: 256,
        O_SYNC: 128,
        O_DSYNC: 4194304,
        O_SYMLINK: 2097152,
        O_NONBLOCK: 4,
        S_IRWXU: 448,
        S_IRUSR: 256,
        S_IWUSR: 128,
        S_IXUSR: 64,
        S_IRWXG: 56,
        S_IRGRP: 32,
        S_IWGRP: 16,
        S_IXGRP: 8,
        S_IRWXO: 7,
        S_IROTH: 4,
        S_IWOTH: 2,
        S_IXOTH: 1,
        F_OK: 0,
        R_OK: 4,
        W_OK: 2,
        X_OK: 1,
        UV_FS_COPYFILE_EXCL: 1,
        COPYFILE_EXCL: 1,
        UV_FS_COPYFILE_FICLONE: 2,
        COPYFILE_FICLONE: 2,
        UV_FS_COPYFILE_FICLONE_FORCE: 4,
        COPYFILE_FICLONE_FORCE: 4
      },
      promises: [Getter],
      gracefulify: [Function: patch]
    }

      at BundleAnalyzerPlugin.getBundleDirFromCompiler (node_modules/webpack-bundle-analyzer/lib/BundleAnalyzerPlugin.js:144:13)
          at Array.map (<anonymous>)

It seems toString() is not a func either.

This output is from my repro example above, so yarn why is just going to give a direct dependency:

yarn why v1.22.10
[1/4] 🤔  Why do we have the module "webpack"...?
[2/4] 🚚  Initialising dependency graph...
[3/4] 🔍  Finding dependency...
[4/4] 🚡  Calculating file sizes...
=> Found "webpack@5.37.0"
info Has been hoisted to "webpack"
info This module exists because it's specified in "dependencies".
info Disk size without dependencies: "5.39MB"
info Disk size with unique dependencies: "8.59MB"
info Disk size with transitive dependencies: "20.3MB"
info Number of shared dependencies: 37
✨  Done in 0.48s.

I've tried using webpack@5.4.0 since that's what this repo tests, but no success either.

@alexander-akait
Copy link
Member

Do you use https://github.com/TypeStrong/fork-ts-checker-webpack-plugin? Maybe you can provide list of plugins?

@kedarv
Copy link
Contributor

kedarv commented May 11, 2021

@alexander-akait can you take a look at my minimal repro example above in this PR: #384 (comment)

No plugins other than this one

@alexander-akait
Copy link
Member

@kedarv

@alexander-akait can you take a look at my minimal repro example above in this PR: #384 (comment)

Wrong link?

@kedarv
Copy link
Contributor

kedarv commented May 11, 2021

I can reproduce this error in Jest. Here's a minimal repro:

bundle.test.js

 import tmp from 'tmp';
 import webpack from 'webpack';
 import BundleAnalyzerPlugin from 'webpack-bundle-analyzer';
 
 function runWebpack(cb) {
    tmp.dir((err, path, cleanupCallback) => {
        const compiler = webpack({
            entry: 'noop-entry.js',
            output: { path },
            plugins: [new BundleAnalyzerPlugin.BundleAnalyzerPlugin()],
        });
        compiler.run(cb);
        cleanupCallback();
    });
 }
 
 describe('some test suite', () => {
    beforeEach(done => {
        runWebpack(err => {
            console.log(err);
            done();
        });
    });

    it('some test', () => {
       console.log("assert something..")
    });
});

(noop-entry.js is just an empty file)

package.json

{
  "name": "test-webpack",
  "version": "1.0.0",
  "main": "index.js",
  "license": "MIT",
  "dependencies": {
    "tmp": "^0.2.1",
    "webpack": "^5.36.2",
    "webpack-bundle-analyzer": "^4.4.1"
  },
  "type": "module",
  "devDependencies": {
    "jest": "^26.6.3"
  },
  "scripts": {
    "test": "jest"
  }  
}

NODE_OPTIONS=--experimental-vm-modules yarn jest results in:

  console.log
    TypeError: Cannot read property 'name' of undefined
        at BundleAnalyzerPlugin.getBundleDirFromCompiler (/Users/kedar/webpack-bundle-analyzer-repro/node_modules/webpack-bundle-analyzer/lib/BundleAnalyzerPlugin.js:144:56)
        at BundleAnalyzerPlugin.startAnalyzerServer (/Users/kedar/webpack-bundle-analyzer-repro/node_modules/webpack-bundle-analyzer/lib/BundleAnalyzerPlugin.js:114:25)
        at /Users/kedar/webpack-bundle-analyzer-repro/node_modules/webpack-bundle-analyzer/lib/BundleAnalyzerPlugin.js:62:33
        at /Users/kedar/webpack-bundle-analyzer-repro/node_modules/webpack-bundle-analyzer/lib/BundleAnalyzerPlugin.js:73:53
        at Array.map (<anonymous>)
        at Immediate.<anonymous> (/Users/kedar/webpack-bundle-analyzer-repro/node_modules/webpack-bundle-analyzer/lib/BundleAnalyzerPlugin.js:73:39)
        at processImmediate (node:internal/timers:464:21)

      at bundle.test.js:20:21

It's unclear why this error is happening right now -- I will note though that this test would pass (ie. Webpack Bundle Analyzer starts up and serves data) under Webpack 4 and the v3 branch of this library.

@alexander-akait @th0r let me know if you'd like me to file an issue instead of adding stuff onto this closed PR.

PS: the tmp part is unnecessary to repro, I just added it here since it's representative of a real test.

Not sure why the link is broken, GitHub web UI bug maybe. I've quoted the original comment

@alexander-akait
Copy link
Member

Give me 5-15 minutes, I will look

@alexander-akait
Copy link
Member

Reproduced, thanks, investigate

@alexander-akait
Copy link
Member

Yes, it is bug, this.compiler.outputFileSystem can be any fs, we only require certain methods (you can look at types https://github.com/webpack/webpack/blob/master/types.d.ts#L7963, as you can see no new(): type)

fix:

  getBundleDirFromCompiler() {
    if (typeof this.compiler.outputFileSystem.constructor === 'undefined') {
      return this.compiler.outputPath;
    }
    
    switch (this.compiler.outputFileSystem.constructor.name) {
      case 'MemoryFileSystem':
        return null;
      // Detect AsyncMFS used by Nuxt 2.5 that replaces webpack's MFS during development
      // Related: #274

      case 'AsyncMFS':
        return null;

      default:
        return this.compiler.outputPath;
    }
  }

@alexander-akait
Copy link
Member

/cc @valscion need your attention

@valscion
Copy link
Member

We would really like to have a failing test case before the change so we won't regress this behavior in the future. I tried making up one, but am unable to do it.

Is the only way possible to replicate this issue in a test by using Jest?

@alexander-akait
Copy link
Member

@valscion Example above #384 (comment) 😕 Just run it and you will see the problem

@alexander-akait
Copy link
Member

And look at types, there is not new(): type, so constructor can be undefined, if we migrate code to ts/jsdocs ts you will see the same problem from typescript

@valscion
Copy link
Member

Yeah, I'm just having a hard time wrapping the reproduction to a test case that I could get it to fail in CI...

@alexander-akait
Copy link
Member

alexander-akait commented May 12, 2021

.cc @valscion hm, put it under todo and will add it late, package is not working, it is more priority

@alexander-akait
Copy link
Member

Also you can use test it using const compiler = webpack(config); compiler.outputFileSystem = require('fs'); webpack.run(callback)

@kedarv
Copy link
Contributor

kedarv commented May 12, 2021

Would be great to add a testcase to prevent regressions in a follow up since this is blocking a webpack 5 migration for me 👍

@kedarv
Copy link
Contributor

kedarv commented May 13, 2021

@alexander-akait : I wrote a PR with your fix in #447
Is it possible to merge and release? Would like to avoid creating an internal fork, but this is preventing me from shipping changes on my end

@valscion
Copy link
Member

#384 (comment)

Also you can use test it using

I tried this one, but maybe I'm doing something wrong as these steps don't give me the failure that I'm supposed to see.

We can go with the PR in #447 but as long as there are no tests, it's possible that we'll break the fix even in a patch release as there is no way we can make sure that this fix doesn't suddenly stop working unless we have test coverage for that.

@valscion
Copy link
Member

The fix in #447 has been released in v4.4.2 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants