webpack-contrib / webpack-bundle-analyzer

Webpack plugin and CLI utility that represents bundle content as convenient interactive zoomable treemap
MIT License
12.58k stars 488 forks source link

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

Closed iamssen closed 3 years ago

iamssen commented 4 years ago

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 commented 4 years ago

CLA assistant check
All committers have signed the CLA.

iamssen commented 3 years ago

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 commented 3 years ago

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:

https://github.com/webpack-contrib/webpack-bundle-analyzer/blob/37f048a500ebaa2394350183dee7b5c4c09d8489/test/plugin.js#L53-L54

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

ludofischer commented 3 years ago

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

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.

kedarv commented 3 years ago

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 commented 3 years ago

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

kedarv commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

@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 commented 3 years ago

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

and run npm ls webpack/yarn why webpack

kedarv commented 3 years ago
  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 commented 3 years ago

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

kedarv commented 3 years ago

@alexander-akait can you take a look at my minimal repro example above in this PR: https://github.com/webpack-contrib/webpack-bundle-analyzer/pull/384#issuecomment-83479228

No plugins other than this one

alexander-akait commented 3 years ago

@kedarv

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

Wrong link?

kedarv commented 3 years ago

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 commented 3 years ago

Give me 5-15 minutes, I will look

alexander-akait commented 3 years ago

Reproduced, thanks, investigate

alexander-akait commented 3 years ago

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 commented 3 years ago

/cc @valscion need your attention

valscion commented 3 years ago

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 commented 3 years ago

@valscion Example above https://github.com/webpack-contrib/webpack-bundle-analyzer/pull/384#issuecomment-838569309 :confused: Just run it and you will see the problem

alexander-akait commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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

alexander-akait commented 3 years ago

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

kedarv commented 3 years ago

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 commented 3 years ago

@alexander-akait : I wrote a PR with your fix in https://github.com/webpack-contrib/webpack-bundle-analyzer/pull/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 commented 3 years ago

https://github.com/webpack-contrib/webpack-bundle-analyzer/pull/384#issuecomment-840081926

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 commented 3 years ago

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