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

Listeners no longer being properly applied to chunks in v1.1.7 #34

Closed
2 of 6 tasks
just-jeb opened this issue Jul 15, 2021 · 7 comments · Fixed by #35
Closed
2 of 6 tasks

Listeners no longer being properly applied to chunks in v1.1.7 #34

just-jeb opened this issue Jul 15, 2021 · 7 comments · Fixed by #35
Assignees
Labels
status: verified The bug issue was reviewed and is verified to have the problem stated. type: bug A bug.

Comments

@just-jeb
Copy link

Type:

  • bug
  • feature
  • enhancement
  • question

Environment:

  • OS:
  • Browser:
  • Library Version:

I'm going to open a PR:

  • yes
  • no

Description:
Hey guys, Kudos for taking ownership on this extension.
Is is working or still WIP? I'm trying to use it with Webpack 5 and it seems to start successfully but it doesn't actually work.
When I change the background.js script the change is not applied.
Moreover, in the original extension I saw a lot of code getting attached to the background.js chunk during the build process and now the only code in chunk is my own.

@just-jeb
Copy link
Author

UPDATE:
I debugged it a bit and here's what I found: the plugin is being triggered but the middleware is not being injected. I've put some console logs inside the injector function like this:

  var matchBgOrContentOrPage = function matchBgOrContentOrPage(name) {
    return name === background || name === contentScript || contentScript && contentScript.includes(name) || name === extensionPage || extensionPage && extensionPage.includes(name);
  };

  return function (assets, chunks) {
    console.log("entered middleware injector", chunks);
    console.log(Array.from(chunks));
    return Array.from(chunks).reduce(function (prev, _ref3) {
      var name = _ref3.name,
          files = _ref3.files;

      console.log(name);
      if (matchBgOrContentOrPage(name)) {
        console.log('!!! Match the Bg', name, files);
        files.forEach(function (entryPoint) {
          console.log("Entry point: ".concat(entryPoint));

and here is the output:

entered middleware injector {
  'main.js': CachedSource {
    _source: ConcatSource { _children: [Array], _isOptimized: false },
    _cachedSourceType: undefined,
    _cachedSource: undefined,
    _cachedBuffer: undefined,
    _cachedSize: undefined,
    _cachedMaps: Map(0) {},
    _cachedHashUpdate: undefined
  },
  'polyfills.js': CachedSource {
    _source: ConcatSource { _children: [Array], _isOptimized: false },
    _cachedSourceType: undefined,
    _cachedSource: undefined,
    _cachedBuffer: undefined,
    _cachedSize: undefined,
    _cachedMaps: Map(0) {},
    _cachedHashUpdate: undefined
  },
  'styles.css': CachedSource {
    _source: ConcatSource { _children: [Array], _isOptimized: false },
    _cachedSourceType: undefined,
    _cachedSource: undefined,
    _cachedBuffer: undefined,
    _cachedSize: undefined,
    _cachedMaps: Map(0) {},
    _cachedHashUpdate: undefined
  },
  'background.js': CachedSource {
    _source: ConcatSource { _children: [Array], _isOptimized: false },
    _cachedSourceType: undefined,
    _cachedSource: undefined,
    _cachedBuffer: undefined,
    _cachedSize: undefined,
    _cachedMaps: Map(0) {},
    _cachedHashUpdate: undefined
  },
  'runtime.js': CachedSource {
    _source: ConcatSource { _children: [Array], _isOptimized: false },
    _cachedSourceType: undefined,
    _cachedSource: undefined,
    _cachedBuffer: undefined,
    _cachedSize: undefined,
    _cachedMaps: Map(0) {},
    _cachedHashUpdate: undefined
  },
  'vendor.js': CachedSource {
    _source: ConcatSource { _children: [Array], _isOptimized: false },
    _cachedSourceType: undefined,
    _cachedSource: undefined,
    _cachedBuffer: undefined,
    _cachedSize: undefined,
    _cachedMaps: Map(0) {},
    _cachedHashUpdate: undefined
  }
}
Array from chunks []

As you can see array that from which we reduce the final source is always empty, therefore the middleware is never applied.
It probably has something to do with the API changes between Webpack 4 and Webpack 5, I assume that in Webpack 4 chunks was an array (or an iterable object), while in Webpack 5 it's no longer the case (or so it seems).
Not familiar with Webpack APIs and how to get name and files from the new API but I think if you fix this then it should fix the issue.

@just-jeb
Copy link
Author

just-jeb commented Jul 15, 2021

I guess this is the breaking change: https://webpack.js.org/blog/2020-10-10-webpack-5-release/#arrays-to-sets.
Although, Array.from(set) should just work, so I'm lost. Not sure what happens here.

UPDATE:
It's getting weirder. I added the following log inside the middlewareInjector:

    console.log(chunks.constructor.name);

and I'm getting Object, rather than Set.
So somehow chunks that are passed into middlewareInjector are not Set (as expected) but an Object.

Any idea?

@rushilsrivastava
Copy link
Member

Hi, thanks for sharing these details. This extension definitely does work, and we use it here at Simplify. Can you share your webpack config as well? We will try to recreate your setup locally and see what we can find!

@rushilsrivastava rushilsrivastava added type: bug A bug. status: needs more info The issue lacks enough info to make progress. labels Jul 15, 2021
@just-jeb
Copy link
Author

Hey, thanks for the swift response! I imagined it works for you 😃. In fact I've tried for hours to see what's wrong with my environment, maybe multiple webpack versions or something else but nothing proved right.

As for your request there are bad news and there are good news.
The bad news is that I unfortunately cannot provide you with a specific webpack config that causes the issue.
The reason is that I'm running it as part of Angular CLI build process and I hook into their process with the missing part of the webpack config (entry for background script and the ExtReloader).
I'm trying to update this tutorial to the newer Angular version that uses Webpack 5, this is how I ended up here.

And now the good news!
I can provide you with the reproduction which you can hook into and see what's wrong. Maybe place a breakpoint and see the configuration yourself. I can't think of any issues with the config, since the plugin is triggered and all the functionality is called, except for the weird part.
But I'm sure you'll do better than me 😄
So here is the branch: https://github.com/just-jeb/angular-chrome-extension/tree/migrate-to-angular-12.
All you need to do is check it out, run npm i and then npm run watch.
If you'll see the background.js in dist folder without the middleware code it means you reproduced the problem and can hook into the process. If everything will work for you then I probably should throw my Mac out the window.
I've already provided the rest of the details, hope for a fast resolution. If you need anything else from me please let me know, I'll do my best to assist you.

@rushilsrivastava
Copy link
Member

rushilsrivastava commented Jul 15, 2021

Thanks for sending that over, can you try using v1.1.6 instead? I believe this is a rare chunk bug we introduced in #23, and should be fixed in the upcoming v1.1.8 release. In the meantime, the deprecated approach in v1.1.6 should work fine, just throw some warnings.

@rushilsrivastava rushilsrivastava added status: verified The bug issue was reviewed and is verified to have the problem stated. and removed status: needs more info The issue lacks enough info to make progress. labels Jul 15, 2021
@rushilsrivastava rushilsrivastava changed the title Is this working? Listeners no longer being properly applied to chunks in v1.1.7 Jul 15, 2021
@rushilsrivastava
Copy link
Member

This has been released in v1.1.8 🎉

@just-jeb
Copy link
Author

just-jeb commented Jul 16, 2021

The fix does the work, thank you! 🥳
I'd love to hear an explanation for the fix (out if curiosity), left a comment on the PR.
Thanks again!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: verified The bug issue was reviewed and is verified to have the problem stated. type: bug A bug.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants