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

improve compatibility with webpack@5 #52

Closed
alexander-akait opened this issue Aug 12, 2020 · 21 comments · Fixed by #54
Closed

improve compatibility with webpack@5 #52

alexander-akait opened this issue Aug 12, 2020 · 21 comments · Fixed by #54

Comments

@alexander-akait
Copy link
Contributor

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
Copy link
Member

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
Copy link
Contributor Author

@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
Copy link
Member

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
Copy link
Contributor Author

@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
Copy link

sokra commented Aug 14, 2020

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
Copy link
Member

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: webpack/webpack-cli#1222 (comment) 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
Copy link
Contributor Author

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

@ryan-roemer
Copy link
Member

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
Copy link
Member

@sokra @evilebottnawi -- I've now got a WIP PR/branch up at: #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
Copy link
Contributor Author

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 added a commit that referenced this issue Oct 15, 2020
- Use non-deprecated hooks / methods of adding assets. Fixes #52 
- Update tests for webpack5 expected failures
@ryan-roemer
Copy link
Member

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

@kelunik
Copy link

kelunik commented Oct 20, 2020

@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
Copy link
Contributor Author

alexander-akait commented Oct 20, 2020

@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
Copy link
Member

@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
Copy link
Member

@kelunik @koconnorIXL -- I have a branch up at bug/webpack5 with a work-in-progress PR at: #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
Copy link
Contributor Author

@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
Copy link
Member

Fixes released in webpack-stats-plugin@1.0.1

@koconnorIXL
Copy link

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

@jdelStrother
Copy link
Contributor

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
Copy link
Member

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

@jdelStrother
Copy link
Contributor

Sure, see #59

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

Successfully merging a pull request may close this issue.

6 participants