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

feat: add support for webpack5 #403

Merged
merged 2 commits into from
Dec 4, 2020

Conversation

mayako21126
Copy link

What kind of change does this PR introduce? (bugfix, feature, docs update, improvement)
feature
What is the current behavior? (You can also link to an open issue here)
it doesn't work with Webpack5
What is the new behavior (if this is a feature change)?
Make it compatible with Webpack5
Does this PR introduce a breaking change?
no
Please check if the PR fulfills contributing guidelines
ok

but some Webpack5 changes caused the test to be error
for example “MultiEntryPlugin”

@mayako21126 mayako21126 marked this pull request as ready for review May 23, 2020 05:51
@mayako21126
Copy link
Author

i think it due to nodejs version conflict

eslint 7.0 need 12.0.0 but .travis is 8.10.0

@mayako21126 mayako21126 force-pushed the add-support-webpack5 branch from 850fce6 to f9fdb66 Compare May 23, 2020 13:05
fix: webpack5 support

style: clean

style: package.json

feat: webpack-5 devDependencies update

fix: conflict of nodejs version

fix: eslint

fix: eslint nvm

fix: eslint bug eslint/eslint#13352

feat: package update
.travis.yml Outdated
@@ -2,7 +2,7 @@ sudo: false
language: node_js

node_js:
- 8.10.0
- 12.0.0
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not using the latest stable LTS version instead?

Suggested change
- 12.0.0
- 12.18.1

https://github.com/nodejs/node/blob/master/doc/changelogs/CHANGELOG_V12.md

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

update 00f4fc2

.travis.yml Outdated
@@ -13,7 +13,8 @@ env:
- ISTANBUL_COVERAGE: yes

script:
- yarn bootstrap --env webpack-4
- nvm install 8.10.0
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it necessary to use a deprecated version and support of highly outdated webpack versions?

Suggested change
- nvm install 8.10.0
- nvm install 8.17.0

@sghoweri
Copy link

Looking forward to seeing this get in!

@rohrlaf
Copy link

rohrlaf commented Jul 10, 2020

@kisenka Is there a plan to merge this PR for upcoming webpack 5 support?

@sghoweri
Copy link

@kisenka Is there a plan to merge this PR for upcoming webpack 5 support?

+1 -- this is the last Webpack library we're using (at least that I'm aware of 🤞 ) that still needs to get updated before we can upgrade to Webpack 5. 👀

@MIreland
Copy link

Likewise- we've been eagerly awaiting this update!

@sghoweri
Copy link

Sorry to bug but any updates on this? What work is left that's preventing this from getting merged down...?!

@ivannovazzi
Copy link

@kisenka can this be at least released in a beta release? It's the only loader remained preventing us from upgrading to webpack 5.

@christophstockinger
Copy link

Great work! I can only recommend the merge, because it solves the problem with Webpack5 compatibility.

Workaround tip, as long as the PR is not merged: Fork the repository of @mayako21126 and create a private NPM package here in Github.

That way I bridge the time when you can work with the package here again.

@ivannovazzi
Copy link

ivannovazzi commented Sep 21, 2020

Webpack 5 officially reaches the release candidate: https://github.com/webpack/webpack/releases/tag/v5.0.0-rc.0

@kisenka It would be nice this PR gets merged

@matthewma7
Copy link

This worked for me. Looking forward to a release version.

@matthewma7
Copy link

This branch has worked great for me with webpack 5.
Webpack 5 is released two days ago.
Would there be an effort to merge this?

@Krasnopir
Copy link

merge please.

@rohrlaf
Copy link

rohrlaf commented Oct 19, 2020

Great work! I can only recommend the merge, because it solves the problem with Webpack5 compatibility.

Workaround tip, as long as the PR is not merged: Fork the repository of @mayako21126 and create a private NPM package here in Github.

That way I bridge the time when you can work with the package here again.

@christophstockinger @mayako21126 I tried to fork the repo with the changes from this PR. Unfortunately, the webpack-5 environment does not start and therefore I can not release a private package. The guide in CONTRIBUTING.md is a little rough. How did you manage to get this to work?

bootstrap

# git clone # from https://github.com/mayako21126/svg-sprite-loader
# git checkout add-support-webpack5

$ sh scripts/build.sh 

#...

Running node v12.18.1 (npm v6.14.5)
yarn install v1.22.4
info No lockfile found.
[1/4] Resolving packages...
warning [email protected]: Deprecated. Please use https://github.com/webpack-contrib/mini-css-extract-plugin
[2/4] Fetching packages...
error [email protected]: The engine "node" is incompatible with this module. Expected version "^10.0.0,^11.0.0,^12.0.0,>=14.0.0". Got "12.18.1"
error Found incompatible module.
info Visit https://yarnpkg.com/en/docs/cli/install for documentation about this command.
webpack-5 env installed

Update: [email protected] breaks compatibility with Node 12 LTS, 5.3.7 was released as a fix.

release

$ yarn release

$ node scripts/build-runtime.js
/.../svg-sprite-loader/scripts/build-examples-dll.js:32
      throw new Error(msgs.join('\n'));
      ^

Error: BUILD EXAMPLES ERRORS
Module not found: Error: Can't resolve 'svg-sprite-loader' in '/.../svg-sprite-loader/examples/browser-sprite-with-dll'

@shameleo
Copy link

@rohrlaf, I don't know what "private package on Github" means, and how to make it, so this is what I did. It may be a bit incorrect or suboptimal, but worked for me.
So I:

  1. made a fork
  2. npm run build:runtime to get *.build.js files in runtime directory
  3. corrected version in package.json to "version": "4.99.99", to avoid any conflicts with official versions
  4. pushed this files to repo
  5. url to repo in package.json:
"svg-sprite-loader": "git+http://[your_host]/svg-sprite-loader-fork.git#[commit_hash]",

Note, my git server uses http.
Maybe that will be ok for you.

@bojanvidanovic
Copy link

Can this be merged?

@rohrlaf
Copy link

rohrlaf commented Nov 6, 2020

@rohrlaf, I don't know what "private package on Github" means, and how to make it, so this is what I did. It may be a bit incorrect or suboptimal, but worked for me.

@shameleo Thanks for your help, I tried to make it work but still not running for me. Replaced svg-sprite-loader with the patch from @mayako21126, but the react-styleguidist will not work.

With "private package" I meant publishing it on a private registry, as we can not alter svg-sprite-loader on the NPM registry.

$ git clone [email protected]:mayako21126/svg-sprite-loader.git
$ git checkout add-support-webpack5
$ npm/yarn install
$ npm run/yarn build:runtime

# change name to "@myorg/svg-sprite-loader" and "publishConfig" in package.json

$ npm login # login into the private registry
$ npm/yarn publish

Build error

Nevertheless, for the node_modules install your HTTP approach or the package publishing should not make a difference. I still can not make the build work, although doing the same steps to generate the runtime files.

With patch by @mayako21126:

(node:16489) [DEP_WEBPACK_COMPILATION_ASSETS] DeprecationWarning: Compilation.assets will be frozen in future, all modifications are deprecated.
BREAKING CHANGE: No more changes should happen to Compilation.assets after sealing the Compilation.
        Do changes to assets earlier, e. g. in Compilation.hooks.processAssets.
        Make sure to select an appropriate stage from Compilation.PROCESS_ASSETS_STAGE_*.

With current [email protected]:

TypeError [ERR_INVALID_ARG_TYPE]: The "data" argument must be of type string or an instance of Buffer, TypedArray, or DataView. Received undefined
TypeError [ERR_INVALID_ARG_TYPE]: The "data" argument must be of type string or an instance of Buffer, TypedArray, or DataView. Received undefined
    at Hash.update (internal/crypto/hash.js:82:11)
    at BulkUpdateDecorator.update (myproject/node_modules/webpack/lib/util/createHash.js:49:14)
    at NormalModule.updateHash (myproject/node_modules/webpack/lib/NormalModule.js:1042:8)
    at Compilation.createModuleHashes (myproject/node_modules/webpack/lib/Compilation.js:2599:12)
    at myproject/node_modules/webpack/lib/Compilation.js:1952:11
    at Hook.eval [as callAsync] (eval at create (myproject/node_modules/webpack/node_modules/tapable/lib/HookCodeFactory.js:33:10), <anonymous>:10:1)
    at Hook.CALL_ASYNC_DELEGATE [as _callAsync] (myproject/node_modules/webpack/node_modules/tapable/lib/Hook.js:18:14)
    at myproject/node_modules/webpack/lib/Compilation.js:1914:36
    at Hook.eval [as callAsync] (eval at create (myproject/node_modules/webpack/node_modules/tapable/lib/HookCodeFactory.js:33:10), <anonymous>:10:1)
    at Hook.CALL_ASYNC_DELEGATE [as _callAsync] (myproject/node_modules/webpack/node_modules/tapable/lib/Hook.js:18:14)
    at Compilation.seal (myproject/node_modules/webpack/lib/Compilation.js:1905:27)
    at myproject/node_modules/webpack/lib/Compiler.js:981:20
    at myproject/node_modules/webpack/lib/Compilation.js:1724:4
    at eval (eval at create (myproject/node_modules/webpack/node_modules/tapable/lib/HookCodeFactory.js:33:10), <anonymous>:48:1)
    at myproject/node_modules/webpack/lib/FlagDependencyExportsPlugin.js:332:11
    at myproject/node_modules/neo-async/async.js:2830:7
error Command failed with exit code 1.

@Eli-Black-Work
Copy link

It looks like there's a #365 pull request that's also supposed to fix this library on Webpack 5. Not sure which pull request is better 🙂

@Eli-Black-Work
Copy link

@Slashgear, Sorry to bother you, but are you still a maintainer for this repository?

@rohrlaf
Copy link

rohrlaf commented Nov 6, 2020

It looks like there's a #365 pull request that's also supposed to fix this library on Webpack 5. Not sure which pull request is better slightly_smiling_face

@Bosch-Eli-Black thanks for pointing this out. #365 is based on [email protected], whereas this PR is based on [email protected]. Nevertheless, both versions cause the same deprecation warning and never finish the build for me: DeprecationWarning: Compilation.assets

@Eli-Black-Work
Copy link

Eli-Black-Work commented Nov 6, 2020

It looks like there's a #365 pull request that's also supposed to fix this library on Webpack 5. Not sure which pull request is better slightly_smiling_face

@Bosch-Eli-Black thanks for pointing this out. #365 is based on [email protected], whereas this PR is based on [email protected]. Nevertheless, both versions cause the same deprecation warning and never finish the build for me: DeprecationWarning: Compilation.assets

Ah, cool; I hadn't noticed that difference.

Are you sure the build is hung and isn't just still running? I know I've seen some people reporting that upgrading to Webpack 5 increased their projects' initial build times. Just curious what might happen if you left the build running for, say, 15 minutes XD

@mayako21126
Copy link
Author

Great work! I can only recommend the merge, because it solves the problem with Webpack5 compatibility.
Workaround tip, as long as the PR is not merged: Fork the repository of @mayako21126 and create a private NPM package here in Github.
That way I bridge the time when you can work with the package here again.

@christophstockinger @mayako21126 I tried to fork the repo with the changes from this PR. Unfortunately, the webpack-5 environment does not start and therefore I can not release a private package. The guide in CONTRIBUTING.md is a little rough. How did you manage to get this to work?

bootstrap

# git clone # from https://github.com/mayako21126/svg-sprite-loader
# git checkout add-support-webpack5

$ sh scripts/build.sh 

#...

Running node v12.18.1 (npm v6.14.5)
yarn install v1.22.4
info No lockfile found.
[1/4] Resolving packages...
warning [email protected]: Deprecated. Please use https://github.com/webpack-contrib/mini-css-extract-plugin
[2/4] Fetching packages...
error [email protected]: The engine "node" is incompatible with this module. Expected version "^10.0.0,^11.0.0,^12.0.0,>=14.0.0". Got "12.18.1"
error Found incompatible module.
info Visit https://yarnpkg.com/en/docs/cli/install for documentation about this command.
webpack-5 env installed

Update: [email protected] breaks compatibility with Node 12 LTS, 5.3.7 was released as a fix.

release

$ yarn release

$ node scripts/build-runtime.js
/.../svg-sprite-loader/scripts/build-examples-dll.js:32
      throw new Error(msgs.join('\n'));
      ^

Error: BUILD EXAMPLES ERRORS
Module not found: Error: Can't resolve 'svg-sprite-loader' in '/.../svg-sprite-loader/examples/browser-sprite-with-dll'

do you installed nvm?
I solved nodejs version conflict with it

@mayako21126
Copy link
Author

image
now work with 5.0.0-rc.6
image

@rohrlaf
Copy link

rohrlaf commented Nov 11, 2020

@mayako21126 @Bosch-Eli-Black, @KingSora and I found the culprit. As we were trying to publish the forked patches for svg-sprite-loader we published the packages in our private registry with @orgname/. As it seems, either Webpack or this loader has problems with scoped package names, but also the Webpack build was not throwing any errors. Now that we use the package without the @orgname/ scope, things run smoothly 😅

@jods4
Copy link

jods4 commented Nov 11, 2020

If this PR is good, I'd love to see it merged and released.
We'd like to move to Webpack 5 here (because of module federation), but this plugin is a blocker.

@Venryx
Copy link

Venryx commented Nov 14, 2020

Now that we use the package without the @orgname/ scope, things run smoothly 😅

What is the full package name for that fork, which works with Webpack 5? (I'd like to use it, until the official svg-sprite-loader package is updated with the fix)

@rohrlaf
Copy link

rohrlaf commented Nov 16, 2020

Now that we use the package without the @orgname/ scope, things run smoothly sweat_smile

What is the full package name for that fork, which works with Webpack 5? (I'd like to use it, until the official svg-sprite-loader package is updated with the fix)

Sorry to disappoint, my company policies did not allow me to publish an open source package. We only have it available from our private registry.

How I did it:

  • apply changes from Add webpack 5 support #365 to latest version 5.0.0 of this loader (only need this loader to be compatible with Webpack 5, so this is shorter than feat: add support for webpack5 #403)
  • npm build:runtime
  • rename loader name in package.json to (prefix)-svg-sprite-loader (as @scope/` does not work)
  • npm login to the registry for upload
  • npm publish to release this package

@ScriptedAlchemy
Copy link

Webpack Maintainer here 👋 can we get some official movement or merges on this project? If dead then i can fork it and maintain a copy. But there needs to be some update since users seem to have patched the problems it had

@ValorLin
Copy link

Webpack Maintainer here 👋 can we get some official movement or merges on this project? If dead then i can fork it and maintain a copy. But there needs to be some update since users seem to have patched the problems it had

Can't wait to see it.

@sghoweri
Copy link

Webpack Maintainer here 👋 can we get some official movement or merges on this project? If dead then i can fork it and maintain a copy. But there needs to be some update since users seem to have patched the problems it had

@ScriptedAlchemy considering the last commit to master was over 6 months ago and (as far as I can tell) there hasn't been any signs of movement on getting this OK'd and merged, I'm starting to think forking might be our only option here 🙁

@d3x42
Copy link
Contributor

d3x42 commented Nov 28, 2020

Hey, folks!

Unfortunately, the maintainer of this project passed away.

Currently, I need some time to get into and check pull requests. I'll start with this PR next week.

@sghoweri
Copy link

@d3x42 Damn. I am so, so sorry to hear that - my sincere condolences. 😞

Take all the time you need (and really appreciate you letting us know).

@Eli-Black-Work
Copy link

That's really sad to hear 😟 @kisenka seemed like a nice guy, on the occasions that I had to talk to him.

@zhongzhong0505
Copy link

Looking forward to seeing this get in!

@ghost
Copy link

ghost commented Dec 4, 2020

I beg of you, just merge this PR already, it's been like half a year.
We have entire projects that can't update to webpack 5 just because of this one loader.

@d3x42 d3x42 merged commit e620de0 into JetBrains:master Dec 4, 2020
@ghost
Copy link

ghost commented Dec 4, 2020

Thank you, very appreciated.

@matthewma7
Copy link

Thank you for merging this but it looks like the CI is stuck and I guess we won't get a release until it passes?

@d3x42
Copy link
Contributor

d3x42 commented Dec 4, 2020

@matthewma7 Yes, build already queued.

@SevenOutman
Copy link

@kisenka Please review and publish a release with this feature.

@ghost
Copy link

ghost commented Dec 5, 2020

@SevenOutman the guy is dead, bro. He's not going to publish anything. Read the message history above.

@SevenOutman
Copy link

@shakhbulatgaz sorry to hear that :(

@d3x42
Copy link
Contributor

d3x42 commented Dec 5, 2020

Currently, I didn't have access to publish it in npm, so I'll try to find someone who could grant it. After that, I'll publish the release.

@Venryx
Copy link

Venryx commented Dec 5, 2020

If the maintainer of an npm package is deceased, NPM support can help transfer ownership to a new maintainer. I saw this happen recently with the firebase-mock package: soumak77/firebase-mock#160

In this case, it may be easier (eg. less of a waiting delay) since the package repo was hosted under an organization, rather than an individual's Github account.

@d3x42
Copy link
Contributor

d3x42 commented Dec 7, 2020

5.1.1 published

@ogonkov
Copy link

ogonkov commented Dec 7, 2020

Great job, but plugin have dependency from incompatible version of html-webpack-plugin. I guess 6.x version should be published instead, with dependencies update.

"html-webpack-plugin": "^3.2.0",

@boomyao
Copy link

boomyao commented Dec 8, 2020

when i upgrade to v5.1.1, this error still exist. @d3x42 @rohrlaf

(node:55321) UnhandledPromiseRejectionWarning: TypeError [ERR_INVALID_ARG_TYPE]: The "data" argument must be one of type string, TypedArray, or DataView. Received type undefined
    at Hash.update (internal/crypto/hash.js:58:11)
    at BulkUpdateDecorator.update (/Users/qiliu/gaoding/delarue/node_modules/webpack/lib/util/createHash.js:49:14)
    at NormalModule.updateHash (/Users/qiliu/gaoding/delarue/node_modules/webpack/lib/NormalModule.js:1048:8)
    at Compilation.createModuleHashes (/Users/qiliu/gaoding/delarue/node_modules/webpack/lib/Compilation.js:2619:12)
    at hooks.optimizeChunkModules.callAsync.err (/Users/qiliu/gaoding/delarue/node_modules/webpack/lib/Compilation.js:1957:11)
    at Hook.eval [as callAsync] (eval at create (/Users/qiliu/gaoding/delarue/node_modules/webpack/node_modules/tapable/lib/HookCodeFactory.js:33:10), <anonymous>:10:1)
    at Hook.CALL_ASYNC_DELEGATE [as _callAsync] (/Users/qiliu/gaoding/delarue/node_modules/webpack/node_modules/tapable/lib/Hook.js:18:14)
    at hooks.optimizeTree.callAsync.err (/Users/qiliu/gaoding/delarue/node_modules/webpack/lib/Compilation.js:1917:36)
    at _err0 (eval at create (/Users/qiliu/gaoding/delarue/node_modules/webpack/node_modules/tapable/lib/HookCodeFactory.js:33:10), <anonymous>:18:1)
    at handleCompilationDonePromise.then (/Users/qiliu/gaoding/delarue/node_modules/html-webpack-plugin/lib/cached-child-compiler.js:237:53)
    at process._tickCallback (internal/process/next_tick.js:68:7)
(node:55321) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). (rejection id: 1)
(node:55321) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.

@d3x42
Copy link
Contributor

d3x42 commented Dec 8, 2020

@ogonkov Fell free to add PR with updates

@mayako21126 mayako21126 mentioned this pull request Dec 10, 2020
@mayako21126
Copy link
Author

Great job, but plugin have dependency from incompatible version of html-webpack-plugin. I guess 6.x version should be published instead, with dependencies update.

"html-webpack-plugin": "^3.2.0",

#422

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

Successfully merging this pull request may close these issues.