bholloway / resolve-url-loader

Webpack loader that resolves relative paths in url() statements based on the original source file
563 stars 71 forks source link

upgrade postcss #169

Closed TrySound closed 3 years ago

TrySound commented 3 years ago

Ref https://github.com/postcss/postcss/releases/tag/8.0.0

Looks like after all we have to drop node 8 support.

TrySound commented 3 years ago

cc @bholloway

bholloway commented 3 years ago

I read "changes to API" and thought the worst, thanks for showing this change is not too painful. 👍

Although I am not sure how to time this. V4 is overdue and it would be nice to ship something compatible with node 8 simply for all those poor souls stuck with old projects.

That said I think keeping on top of postcss updates is a good thing so it would be a quick followup if delayed.

TrySound commented 3 years ago

Yeah, probably v4 can be released with just adjust loader upgrade and then v5 with new schema utils and postcss.

bholloway commented 3 years ago

Webpack 5 requires 10 as minimum and just looking at the v5 project it already has some related tasks

So I will add this to v5.

ArnaudLier commented 3 years ago

Is there any workaround atm?

TrySound commented 3 years ago

@ZeProf2Code Why? Do you have problem with postcss 7?

ArnaudLier commented 3 years ago

I want to upgrade to TailwindCSS 2 and I need PostCSS 8.

TrySound commented 3 years ago

AFAIK in this package postcss is used only internally. It should not affect upgrade on your side.

brian-kephart commented 3 years ago

I'm having the same difficulty as @ZeProf2Code when attempting to upgrade PostCSS to version 8. Webpack gets a ton of compile errors pointing back to this package. It seems like PostCSS 7/8 don't coexist peacefully in this context.

TrySound commented 3 years ago

Which errors? Might be unrelated problem.

dvdknaap commented 3 years ago

Is there any update because more and more packages are requiring postcss 8 ?

dvdknaap commented 3 years ago

@bholloway when i force the version 4 and postcss 8 everything is working on my end

yarn add resolve-url-loader@4.0.0-alpha.3 --dev yarn add postcss@^8.0.0 --dev

edit: without the postcss force you still get the error that postcss 8 is required for fontawesome so i guess it's still dependancy issue

bholloway commented 3 years ago

@dvdknaap I've been thinking more on this.

I could probably put a semver comparitor inthat allows postcss@8.

"postcss": ">=7.0.35 <9",

I'm not sure that it would drop support for node 8 per sec, since anyone on node 8 could still use postcss@7 right?. That would allow us to address the issue in V4.

Does that sound like it would work? I mean, the work done by @TrySound is awesome but AFAIK we should be able to use postcss@8 for a while with the old API right?

EDIT - This doesn't work as I would have hoped. The engines check will not bump the installed version down, it will just fail install. 😞

jmfrancois commented 3 years ago

It could also be a peer dependencies ? so the project decide the version it uses and you try to ensure compatiblity with as many versions as you want on your side ?

On our side we need to upgrade to postcss 8 because of security fix provided in last release (8.2.10) and this is the last dependency I have which rely on postcss 7 without ugprade option (except force resolution)

Fixed ReDoS vulnerabilities in source map parsing.

ErideonTech commented 3 years ago

https://nvd.nist.gov/vuln/detail/CVE-2021-23368

resolve-url-loader is a package used by Angular (compiler only), hence flags as a security issue for us Thanks for updating

bholloway commented 3 years ago

Okay let me see what we can do.

~Please lets move to the pinned discussion issue to discuss further.~ ~EDIT - my mistake there is no discussion issue.~ EDIT - #198 is now the discussion issue.

In the mean time please consider the workaround @dvdknaap suggested above.

bholloway commented 3 years ago

For some reason this is failing hard in E2E tests. As soon as I work out the issue I will merge the PR. If there is any additional work I will do that on the v5 branch rather than asking for more changes here.

@TrySound et. al. FYI for v5 I intend to require node@>=12 and to push all the dependencies to the latest.