FormidableLabs / webpack-stats-plugin

Webpack stats plugin for build information, file manifests, etc.
MIT License
353 stars 33 forks source link

improve compatibility with webpack@5 #52

Closed alexander-akait closed 4 years ago

alexander-akait commented 4 years ago

in webpack@5 adding assets on emit hook is deprecated, so it should be improve, we talked with @sokra about migration and compatibility.

You can always get the stats of the current state via compilation.getStats, but I would not consider the stats.json as part of the assets/output files generated by webpack. It should use the done hook and write stats.json directly to the compilation.intermediateFileSystem (instead of the outputFileSystem).

So, we need do migration on done hook and use compilation.intermediateFileSystem

Thanks for plugin.

P.S. I would also like to say that it is very popular and we would to get it in webpack-contrib as official plugin, if you interested in this, just say me it and we will start to do it.

ryan-roemer commented 4 years ago

Hi @evilebottnawi ! 👋

I've started a branch on this at https://github.com/FormidableLabs/webpack-stats-plugin/tree/feature/webpack5 and I've got my test harness in place for all of webpacks 1 - 5 now (using beta 25) and everything works as-is.

For migrating to the new format do you have any examples? I was able to access the intermediateFileSystem fs methods but they're not working the way I would expect (I'm having path resolution errors). Do you have any examples of adding assets in a done hook using IFS? Thanks!

alexander-akait commented 4 years ago

@ryan-roemer Hello, you can't add asset to compilation.assets in done hook, I think it is unnecessary, does it exist when you need stats file for memory filesystem?

/cc @sokra

ryan-roemer commented 4 years ago

So right now in emit hook/event I'm doing this:

        curCompiler.assets[filename] = {
          source() {
            return statsStr;
          },
          size() {
            return statsStr.length;
          }
        };

where filename is relative to config output.path. Instead, I was planning on stashing statsStr and outputting it later through whatever means during the done hook, but I'm just not sure what that looks like. (I took a stab before, but the relative paths of filename weren't matching up correctly or something). I didn't get that far before turning to ask for help, because it's probably just easiest to see a proper use of IFS with the done hook in action....

Do you or @sokra have an example of using the done hook to similarly output a file as part of compilation and to the same output (whether memfs or real fs) so that wherever your JS assets go, the outputted JSON file goes as well?

Thanks!

alexander-akait commented 4 years ago

@ryan-roemer You can use getAssetPath https://github.com/webpack/webpack/blob/master/lib/Compilation.js#L3038 to get right path for filename

I can send a PR late with example, do we really need webpack@3 supporting? Because it was deprecated and unsupported?

sokra commented 4 years ago

Ok I see multiple use cases for this plugin:

  1. You want a complete stats.json for further analytics e. g. webpack-bundle-analyser or other tools of this kind.

  2. You want a partial stats.json with specific info for usage by some server-side code e. g. to create the HTML file with the correct script tags.

  3. You want a partial stats.json for usage by some client-side code e. g. to send the build hash to analytics or use the filename for prefetching or service worker.

And I see 2 way the plugin could work:

A. It adds the stats.json as asset to the compilation, it's displayed in the final stats output and emitted by the Compiler. When the output filesystem is changed (e. g. by the webpack-dev-server) the stats.json is written to this filesystem. For the webpack-dev-server this means the stats.json is not written to disk but instead served by the server directly. The asset is written to output.path.

B. It writes the stats.json to the intermediateFileSystem when the compilation has finished (done hook). This filesystem is for build-related output, e. g. it's used to write records or profiling data. It's (usually) not changed. With the webpack-dev-server this would always write the stats.json to the disk. Files in the intermediateFileSystem do usually not use output.path and the full path (or relative to process.cwd()) is usually provided to the plugin.

I think for use case 1 and 2 you want way B, for use case 3 you want way A.


Instead of compilation.assets[filename] = { source, size } you can use compilation.emitAsset(filename, new RawSource(statsStr)). That would also be more correct, since size should be in bytes not in chars. You find RawSource in the webpack-sources npm package.

ryan-roemer commented 4 years ago

Thanks for the in-depth comment @sokra !

@evilebottnawi -- I've got test fixtures mostly up and running, but I'm currently blocked by webpack-cli@4.0.0-beta.8 because my test config is:

  entry: {
    main: "../../src/main.js"
  },

but with webpack-cli@4.0.0-beta.8 that entry config isn't being used and instead, it's finding the library itself one level up at ../../../index.js which mucks up all my testing with using an incorrect entry point. Reading through webpack-cli issues I think I'm hitting this issue: https://github.com/webpack/webpack-cli/issues/1222#issuecomment-665555817 that looks like it's fixed in next but not published yet in webpack-cli.

Once webpack-cli and my webpack builds with webpack5 + webpack-cli@4.0.0-beta start working with my entry point situtation, I'll resume work. Thanks!!!

alexander-akait commented 4 years ago

@ryan-roemer Thanks, new release will be soon (today/tomorrow)

ryan-roemer commented 4 years ago

Hi @sokra -- Now that webpack-cli has been released and unblocked me, I've got my webpack5 test suite passing via the deprecated method and am turning back to upgrade this plugin.

I'd ideally like just one place that emits stats and emits them as completely as possible (I don't mind if say later plugins then emit assets that get missed). So, if I want to do the compilation.emitAsset(filename, new RawSource(statsStr)) approach (and avoid intermediateFileSystem), where's the appropriate hook to do this? Is it done or something else?

Thanks!

ryan-roemer commented 4 years ago

@sokra @evilebottnawi -- I've now got a WIP PR/branch up at: https://github.com/FormidableLabs/webpack-stats-plugin/pull/54

I'm going for this for the modern webpack5 hook:

          // Modern: `processAssets` is one of the last hooks before frozen assets.
          compilation.hooks.processAssets.tapPromise(
            {
              name: "stats-writer-plugin",
              stage: compilation.PROCESS_ASSETS_STAGE_DERIVED
            },
            () => this.emitStats(compilation)
          );

but I'm not totally sure PROCESS_ASSETS_STAGE_DERIVED is the best stage, but it does avoid the deprecation warning (I reviewed through the webpack code to find it and the docs). Seems right, but this is all a bit unfamiliar to me...

... and this is how I'm emitting if the compiler has emitAsset():

        if (curCompiler.emitAsset) {
          // Modern method.
          curCompiler.emitAsset(filename, source);
        } else {
          // Fallback to deprecated method.
          curCompiler.assets[filename] = source;
        }
alexander-akait commented 4 years ago

Drop old versions of webpack and use curCompiler.emitAsset always, no need to support 4.10.0/4.11.0/etc

Yes, you need to use compilation.hooks.processAssets if you need to adding assets

ryan-roemer commented 4 years ago

Released in webpack-stats-plugin@1.0.0. Thanks for the help @evilebottnawi @sokra !

kelunik commented 4 years ago

@ryan-roemer Thanks for the update! Unfortunately, this results in Conflict: Multiple assets emit different content to the same filename stats.json for me. This happens, because the processAssets hook is called multiple times.

alexander-akait commented 4 years ago

@ryan-roemer https://github.com/FormidableLabs/webpack-stats-plugin/blob/main/lib/stats-writer-plugin.js#L75

compiler.hooks.thisCompilation.tap("stats-writer-plugin", (compilation) => {
  // Logic
});

Otherwise you will be work on each child compilation, you don't need it

ryan-roemer commented 4 years ago

@evilebottnawi -- Thanks I'll try that out! Other question if you're around -- is there a better explanation of the stages available for processAssets hook? In particular which one is the latest one in the build to call?

@kelunik -- I'll get a brach up soon for you to try with @evilebottnawi's sugggestion

ryan-roemer commented 4 years ago

@kelunik @koconnorIXL -- I have a branch up at bug/webpack5 with a work-in-progress PR at: https://github.com/FormidableLabs/webpack-stats-plugin/pull/58 if anyone can comment on if this addresses the multiple processAssets calls issue. (I'm working on other webpack5-integration issues introduced in 1.0.0 in that branch as well)

Thanks!

alexander-akait commented 4 years ago

@ryan-roemer

Thanks I'll try that out! Other question if you're around -- is there a better explanation of the stages available for processAssets hook? In particular which one is the latest one in the build to call?

I think this is good https://github.com/webpack/webpack/blob/master/lib/Compilation.js#L3463

ryan-roemer commented 4 years ago

Fixes released in webpack-stats-plugin@1.0.1

koconnorIXL commented 4 years ago

@ryan-roemer - All of my problems seem resolved after your changes

jdelStrother commented 3 years ago

I'm still on webpack 4, and on upgrading from webpack-stats-plugin 0.3.2 have started seeing "WARNING in Conflict: Multiple assets emit different content to the same filename stats.json".

I'm using these webpack packages:

    "webpack": "^4.44.2",
    "webpack-cli": "^3.3.12",
    "webpack-dev-server": "^3.11.0",
    "webpack-stats-plugin": "^1.0.1"
ryan-roemer commented 3 years ago

@jdelStrother -- Can you create a minimal repository with install and build steps to produce the issue? Thanks!

jdelStrother commented 3 years ago

Sure, see #59