webcompat / webcompat-reporter-extensions

Browser extensions to help report site compatibilty issues.
26 stars 21 forks source link

Fixes #95. Update npm dependencies. #97

Closed laghee closed 6 years ago

miketaylr commented 6 years ago

@laghee thanks!

I see the tests exploding on Travis, https://travis-ci.org/webcompat/webcompat-reporter-extensions/builds/347918793?utm_source=github_status&utm_medium=notification

After you updated package.json locally, did you re-run npm install? Do tests still pass for you if you do that?

miketaylr commented 6 years ago

(in case it's useful, here's the differences from webpack 3 to 4... https://github.com/webpack/webpack/releases/tag/v4.0.0)

(and in case this turns out to be too hairy, we can update everything except webpack, and file a follow up issue to upgrade to webpack 4 -- which isn't super urgent until we need a feature that it provides)

laghee commented 6 years ago

@miketaylr Yikes. Yes, when I npm install, there is definite test ugliness. The first one passes, but the rest fail spectacularly. Judging from the errors, none of the dist assets are being created now.

I went through the big changes for v4, and the only one that pops out (to someone with a hazy grasp on module systems) is the issue about importing CommonJS (https://medium.com/webpack/webpack-4-import-and-commonjs-d619d626b655). I see that the browser/addon.js files might be doing this? (https://github.com/webcompat/webcompat-reporter-extensions/search?utf8=%E2%9C%93&q=import&type=code)

miketaylr commented 6 years ago

@miketaylr Yikes. Yes, when I npm install, there is definite test ugliness. The first one passes, but the rest fail spectacularly. Judging from the errors, none of the dist assets are being created now.

Yep! That's what I see too.

the only one that pops out (to someone with a hazy grasp on module systems) is the issue about importing CommonJS

Our module imports shouldn't be affected, because we're using ES6 modules (ESM), which isn't the same thing as CommonJS modules.

Here's a small tip: try to create the dist assets for a single browser w/ webpack 4 just via the commandline:

npx webpack --config firefox/webpack.config.js

(I get prompted to install a new webpack-cli package, so go ahead and install that, and then try to follow the next rabbit hole after trying the command again🐰.. :))

Those errors might be more helpful than the test ones -- I think Intern (our test runner) ends up swallowing a lot of useful error messages.

laghee commented 6 years ago

Now I'm confused. This looks like it's ... working? (The mode warning, from what I understand, isn't fatal.) Unless it's supposed to be creating chunks outside of main? screenshot 2018-03-02 17 37 10

miketaylr commented 6 years ago

Huh, interesting....

I get the following error (after I got past the webpack-cli warning -- did you have to install that @laghee?)

npx webpack --config firefox/webpack.config.js
(node:23008) DeprecationWarning: Tapable.plugin is deprecated. Use new API on `.hooks` instead
/Users/mitaylor/dev/webcompat-reporter-extensions/node_modules/copy-webpack-plugin/dist/index.js:141
                    compilation.fileDependencies.push(file);
                                                 ^

TypeError: compilation.fileDependencies.push is not a function
    at /Users/mitaylor/dev/webcompat-reporter-extensions/node_modules/copy-webpack-plugin/dist/index.js:141:50
    at arrayEach (/Users/mitaylor/dev/webcompat-reporter-extensions/node_modules/lodash/lodash.js:537:11)
    at Function.forEach (/Users/mitaylor/dev/webcompat-reporter-extensions/node_modules/lodash/lodash.js:9359:14)
    at /Users/mitaylor/dev/webcompat-reporter-extensions/node_modules/copy-webpack-plugin/dist/index.js:136:30
    at AsyncSeriesHook.eval [as callAsync] (eval at create (/Users/mitaylor/dev/webcompat-reporter-extensions/node_modules/webpack/node_modules/tapable/lib/HookCodeFactory.js:24:12), <anonymous>:7:1)
    at AsyncSeriesHook.lazyCompileHook [as _callAsync] (/Users/mitaylor/dev/webcompat-reporter-extensions/node_modules/webpack/node_modules/tapable/lib/Hook.js:35:21)
    at asyncLib.forEach.err (/Users/mitaylor/dev/webcompat-reporter-extensions/node_modules/webpack/lib/Compiler.js:278:27)
    at done (/Users/mitaylor/dev/webcompat-reporter-extensions/node_modules/neo-async/async.js:2809:11)
    at /Users/mitaylor/dev/webcompat-reporter-extensions/node_modules/neo-async/async.js:2760:7
    at /Users/mitaylor/dev/webcompat-reporter-extensions/node_modules/graceful-fs/graceful-fs.js:43:10
    at FSReqWrap.oncomplete (fs.js:135:15)
miketaylr commented 6 years ago

Hmm, I see there's a new version of the copy-webpack-plugin: https://www.npmjs.com/package/copy-webpack-plugin, but I don't have that installed.

I assume you have 4.3.1 installed as well?

npm ls | grep copy-webpack
├─┬ copy-webpack-plugin@4.3.1
laghee commented 6 years ago

@miketaylr I did install webpack-cli first thing after the failed tests in case it was somehow related (optimism), and I have 4.3.1 installed for the copy-webpack-plugin as well (I ran ncu again, just in case I somehow did something weird along the way).

Here's another weird thing: When I run npm webpack -v, I get back 3.10.10 ... but it's upgraded in the package.json file.

miketaylr commented 6 years ago

Here's another weird thing: When I run npm webpack -v, I get back 3.10.10 ... but it's upgraded in the package.json file.

Hrm. yeah. That's weird.

So just messing around here, once I install webpack-cli, and upgrade the copy-plugin to 4.5.0, it seems to work w/ v4?

Try to rm -rf node_modules/, and reinstall.

(uh, i'm wilfully ignoring the npm ERR! peer dep missing: webpack@^3.1.0, required by extract-text-webpack-plugin@3.0.2 error)

miketaylr commented 6 years ago

I guess https://github.com/webpack-contrib/copy-webpack-plugin/issues/225 solved the error I see when using webpack 4 and the 4.3.1 version of the plugin. Heh, fixed 20 hours ago -- we're super on the bleeding edge.

laghee commented 6 years ago

Exciting!

Aaargh. No change. Still failing last 3 tests, but npx works. Maybe I should start fresh on a new branch ...

You just updated the copy-webpack-plugin itself, not the dependency required, correct?

miketaylr commented 6 years ago

Maybe I should start fresh on a new branch ...

Another option is to land all the updates now (except the webpack 4 one), and file a new issue for "Migrate to webpack 4", which seems like it needs a bit more investigation. And if that's interesting to you, that issue is all yours. Or we can leave it to bake for a bit. Up to you! Either option works for me.

laghee commented 6 years ago

@miketaylr OK, I'm happy to do that (separate out webpack 4 and work on it separately).

I went ahead and just pushed another commit, but if you'd like me to do some sort of wacky rebase/squash maneuver, let me know!

miketaylr commented 6 years ago

Perfect, thanks! I'll just use the squash and merge button, no need for git acrobatics just yet.