postcss / postcss-import

PostCSS plugin to inline at-import rules content
MIT License
1.38k stars 115 forks source link

Allow urls that contain fragments, fixes #560 #561

Closed romainmenke closed 8 months ago

romainmenke commented 9 months ago

The intention behind ignoring urls with a fragment was to make the plugin more correct. The thinking at the time was that these are never valid imports for a filesystem. With nodejs subpath imports that assumption is incorrect.

romainmenke commented 8 months ago

@RyanZim could you take a look at this when you have time? It is not very urgent given that Vite applies a patch for now, but I don't want to forget about this :) Thank you

RyanZim commented 8 months ago

Apologies, I've been incredibly busy. This is technically a breaking change, right? Code LGTM.

romainmenke commented 8 months ago

I don't think this is a breaking change :)

Import statements with fragments were ignored before this change. Effectively resulting in left over @import in the middle of your CSS file. This would be ignored by browsers as @import needs to come first.

This would be a silent failure. But it is extremely unlikely that users started doing this since v14 was released.

After this change postcss-import will try to resolve the file. Unless you have special handling, like Vite does, you will get a hard error on a missing file.

This is no longer a silent failure but users can trivially resolve this by removing the incorrect @import.

I changed it in v14 was because it is more correct in a bundler context when only considering the CSS specification. I wasn't aware of how tools like Vite used this.


Edit:

Always possible that I overlooked something :)

RyanZim commented 8 months ago

Thanks for the detailed explanation. I think it's still technically a breaking change, key word being technically, but I agree it probably doesn't affect any real-world use, so probably safe to release without a major bump.

RyanZim commented 8 months ago

I'm a fairly strong believer in "never ship on Fridays," so will be sometime next week until this is released.

RyanZim commented 8 months ago

Published v16.1.0

romainmenke commented 8 months ago

Thank you @RyanZim 🙇