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

Incompatible with hard-source-webpack-plugin #148

Closed
ctrlplusb opened this issue Dec 8, 2016 · 19 comments
Closed

Incompatible with hard-source-webpack-plugin #148

ctrlplusb opened this issue Dec 8, 2016 · 19 comments

Comments

@ctrlplusb
Copy link
Contributor

I have logged this on hard-source-webpack-plugin here, but I am going to log an issue here too.

When this plugin is used alongside hard-source-webpack-plugin I get the following error on the second build of the project:

Error: offline-plugin: Plugin's runtime wasn't added to one of your bundle entries. See this https://goo.gl/YwewYp for details.
at Compiler. (/Users/foo/bar/baz/node_modules/offline-plugin/lib/index.js:279:20)

The second attempt I assume tries to use the cache generated by hard-source-webpack-plugin.

@NekR
Copy link
Owner

NekR commented Dec 8, 2016

@ctrlplusb hmm, if hard-source-webpack-plugin doesn't re-emit runtime.js (because its empty most of the time empty and doesn't change) then offline-plugin isn't able to hijack it and verify that runtime is used. Possible solution is to not through when runtime isn't used, but this will hurt the UX. Maybe also makes sense to have random query param to bust caching, like for old good GET requests on the web :-)

@mzgoddard
Copy link

One thought I have for a solution for this is to search for the runtime module during emit.

      compiler.plugin('emit', function (compilation, callback) {
        var runtimeAdded;
        function walk(compilation) {
          compilation.modules.forEach(function(module) {
            if (module.resource === runtimePath) {
              runtimeAdded = true;
            }
          });
          compilation.children.forEach(walk);
        }
        walk(compilation);

        if (!runtimeAdded && !_this2.__tests.ignoreRuntime) {

hard-source re-emits the runtime module but doesn't go through the NormalModuleFactories build path instead opting to skip all that for speed. (The point where the flag is recorded currently is during resolving and resolving is one of webpack's slower steps and where HardSource finds a lot of improvement for projects.) The module from hard-source has the resource attribute like normal HardModules so instead of tagging when the module is considered we could consider during emit by checking all the modules built by all compilations.

@NekR
Copy link
Owner

NekR commented Dec 8, 2016

@mzgoddard okay, sounds like a way to go. Do you think this is the only/best way to handle that? Will it affect normal compilations (without hard-source-webpack-plugin) somehow?

@mzgoddard
Copy link

Do you think this is the only/best way to handle that? Will it affect normal compilations (without hard-source-webpack-plugin) somehow?

I thought about that a bunch yesterday. I have one small improvement. Iterate over the chunks and modules in them.

function walk(compilation) {
  compilation.chunks.forEach(function(chunk) {
    chunk.modules.forEach(function(module) {
      if (module.resource === runtimePath) {
        offlinePlugin.flags.runtimeAdded = true;
      }
    });
  });
  compilation.children.forEach(walk);
}
walk(compilation);

It works with hard-source. And it works with Prefetch plugin. I'm trying to think about interaction with other plugins but one thing I thought of was the current test, only tests that the runtime module is built. Not whether its included in a chunk that was produced. PrefetchPlugin leads to a module and its dependencies being built without any of them needing to be required from modules built and emitted as part of a chunk. So the current test can give a false positive when used with a prefetch on the runtime or a module that depends on the runtime.

I wrote up a repo to test these ideas along with a little write up on the issue. https://github.com/mzgoddard/webpack-hard-source-offline-compat-demo

I'm also wondering about false negatives. DllReferencePlugin comes to mind. You could build a Dll with DllPlugin and OfflinePlugin. Then build with Reference and Offline. That setup should work. The current test and the proposed one would both fail here though. Pulling out the info to see if the runtime is somewhere in that second build may be tricky.

@kikoanis
Copy link

I noticed this only happens when using webpack 2.1.0-beta.27 or 2.1.0-beta.28 but NOT 2.1.0-beta.25

Not sure if the issue reported by @ctrlplusb is really related to offline-plugin or hard-source-webpack-plugin

Just my 2 cents.

@mzgoddard
Copy link

@kikoanis Are you experiencing this bug with webpack@2.1.0-beta.28 (or webpack@2.1.0-beta.27)?

Error: offline-plugin: Plugin's runtime wasn't added to one of your bundle entries. See this https://goo.gl/YwewYp for details.
at Compiler. (/Users/foo/bar/baz/node_modules/offline-plugin/lib/index.js:279:20)

I think we have identified the specific issue between offline-plugin@4 and hard-source-webpack-plugin and it happens with webpack@1 or any of the beta versions.

You might have experienced the bug in hard-source that existed for a few hours yesterday when webpack@2.1.0-beta.28 was released. Thats fixed in hard-source-webpack-plugin@0.3.3.

@NekR
Copy link
Owner

NekR commented Dec 14, 2016

I'm not sure I follow now. Where is the bug now, in webpack? If it isn't in webpack when why it exists in some versions and doesn't in others?

@kikoanis
Copy link

@mzgoddard, in both but not in webpack@2.1.0-beta.25.

I think it's nothing to do with both plugins and more to do with webpack beta releases. I need to do more debugging though to be sure.

@mzgoddard
Copy link

@kikoanis I'm not sure how webpack@2.1.0-beta.25 is passing for you. If I turn off the compat plugin in this reproduction repo https://github.com/mzgoddard/webpack-hard-source-offline-compat-demo webpack@2.1.0-beta.25 still has the error for me.

@kikoanis
Copy link

Well I tested it on a private repo and I can assure you it is passing (both of the plugins are the latest version). Even after installing 27/28 and then back to reinstalling 25. So nothing to do with something left behind.

May be different configuration! I do not know. Will have to check it against your repo when I have sometime and let you know.

@NekR
Copy link
Owner

NekR commented Dec 14, 2016

I think the simplest and the fastest they to kind of fix it is to simply not do the check when HardSource and/or Prefetch plugins are used. This at least will make it pass without errors.

I'm also wondering about false negatives. DllReferencePlugin comes to mind. You could build a Dll with DllPlugin and OfflinePlugin.

offline-plugin doesn't really work with DLL and I don't yet know why, see: #125. Hopefully almost no one uses DLL in production, but rather only in dev.

https://github.com/mzgoddard/webpack-hard-source-offline-compat-demo

I'm not sure if I want to remove current test completely. After all, that test is legit unless some other plugins modify build process. i.e. I might expect it to be broken with hyperpack plugin too.

@mzgoddard Maybe I can use your test when current test fails, i.e. fallback to yours. Also false positives/negatives worry me here.

Maybe it makes to just turn this check into a warning?

@NekR
Copy link
Owner

NekR commented Dec 28, 2016

@mzgoddard what do you about this: #152? Might be incompatible with another plugin?

@MoOx
Copy link
Contributor

MoOx commented Jan 28, 2017

@NekR the issue I am facing with Phenomic is related to this thread.
BUT I am facing this issue with webpack 1. So latest of hard-souce-* + offline-plugin@^4. poke @mzgoddard.

To reproduce

git clone https://github.com/MoOx/phenomic.git
cd phenomic
npm install
cd docs
npm run build
npm run build

Running 2 builds to generate the cache, then trigger the issue.

@NekR
Copy link
Owner

NekR commented Jan 29, 2017

@MoOx Yeah, I see, it has to be fixed. I can do that tomorrow, I think.

NekR added a commit that referenced this issue Jan 31, 2017
... and use compilation.fileDependencies to look for runtime-template.js file usage
Fixes #148
@NekR
Copy link
Owner

NekR commented Jan 31, 2017

Okay, I made a fix for this: #172
@mzgoddard would you mind to review it?

@mzgoddard
Copy link

mzgoddard commented Feb 1, 2017 via email

@NekR
Copy link
Owner

NekR commented Feb 1, 2017

@mzgoddard thanks! There is just a couple lines of changes and I already tests with hard-source-webpack-plugin, seems to work fine. Just wondering if it would work fine with other plugins such PrefetchPlugin which you mentioned here.

@NekR
Copy link
Owner

NekR commented Feb 3, 2017

This is fixed, I just published 4.6.0. Thanks for helping everyone!

@NekR NekR closed this as completed Feb 3, 2017
@NekR
Copy link
Owner

NekR commented Feb 4, 2017

@MoOx btw, does it works now for you?

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

No branches or pull requests

5 participants