search

JetBrains / svg-sprite-loader

Webpack loader for creating SVG sprites.
MIT License
2.02k stars 270 forks source link

feat: add support for webpack5 #403

Closed mayako21126 closed 3 years ago

mayako21126 commented 4 years ago

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 commented 4 years ago

i think it due to nodejs version conflict

eslint 7.0 need 12.0.0 but .travis is 8.10.0

sghoweri commented 4 years ago

Looking forward to seeing this get in!

rohrlaf commented 4 years ago

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

sghoweri commented 4 years ago

@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 commented 4 years ago

Likewise- we've been eagerly awaiting this update!

sghoweri commented 4 years ago

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

ivannovazzi commented 4 years ago

@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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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

matthewma7 commented 3 years ago

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 commented 3 years ago

merge please.

rohrlaf commented 3 years ago

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 extract-text-webpack-plugin@3.0.2: Deprecated. Please use https://github.com/webpack-contrib/mini-css-extract-plugin
[2/4] Fetching packages...
error terser@5.3.6: 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: terser@5.3.6 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 commented 3 years ago

@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 3) pushed this files to repo 4) 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 commented 3 years ago

Can this be merged?

rohrlaf commented 3 years ago

@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 git@github.com: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 svg-sprite-loader@5.0.0:

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 commented 3 years ago

It looks like there's a https://github.com/JetBrains/svg-sprite-loader/pull/365 pull request that's also supposed to fix this library on Webpack 5. Not sure which pull request is better 🙂

Eli-Black-Work commented 3 years ago

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

rohrlaf commented 3 years ago

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 svg-sprite-loader@4.1.6, whereas this PR is based on svg-sprite-loader@5.0.0. Nevertheless, both versions cause the same deprecation warning and never finish the build for me: DeprecationWarning: Compilation.assets

Eli-Black-Work commented 3 years ago

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 svg-sprite-loader@4.1.6, whereas this PR is based on svg-sprite-loader@5.0.0. 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 commented 3 years ago

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 extract-text-webpack-plugin@3.0.2: Deprecated. Please use https://github.com/webpack-contrib/mini-css-extract-plugin
[2/4] Fetching packages...
error terser@5.3.6: 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: terser@5.3.6 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 commented 3 years ago

image now work with 5.0.0-rc.6 image

rohrlaf commented 3 years ago

@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 :sweat_smile:

jods4 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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:

ScriptedAlchemy commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

@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 commented 3 years ago

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

zhongzhong0505 commented 3 years ago

Looking forward to seeing this get in!

ghost commented 3 years ago

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.

ghost commented 3 years ago

Thank you, very appreciated.

matthewma7 commented 3 years ago

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 commented 3 years ago

@matthewma7 Yes, build already queued.

SevenOutman commented 3 years ago

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

ghost commented 3 years ago

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

SevenOutman commented 3 years ago

@shakhbulatgaz sorry to hear that :(

d3x42 commented 3 years ago

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 commented 3 years ago

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: https://github.com/soumak77/firebase-mock/issues/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 commented 3 years ago

5.1.1 published

ogonkov commented 3 years ago

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.

https://github.com/JetBrains/svg-sprite-loader/blob/d5ddfef3784fa179a57fd35f2f3ebb3a9064c076/package.json#L38

boomyao commented 3 years ago

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 commented 3 years ago

@ogonkov Fell free to add PR with updates

mayako21126 commented 3 years ago

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.

https://github.com/JetBrains/svg-sprite-loader/blob/d5ddfef3784fa179a57fd35f2f3ebb3a9064c076/package.json#L38

https://github.com/JetBrains/svg-sprite-loader/pull/422