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

Support a way to process a URL with fragments #560

Closed
sapphi-red opened this issue Feb 20, 2024 · 8 comments
Closed

Support a way to process a URL with fragments #560

sapphi-red opened this issue Feb 20, 2024 · 8 comments

Comments

@sapphi-red
Copy link

sapphi-red commented Feb 20, 2024

In Vite, we were resolving @import '#foo' with subpath imports. But since postcss 16.0.0 (#539), these URLs are filtered out before it reaches any code that the plugin user side can control. That made Vite not able to resolve these URLs (vitejs/vite#15861).

Would it be possible to add an option to skip this default behavior?

@romainmenke
Copy link
Collaborator

Hi @sapphi-red,

Can you give more info on the use cases?

Is there an alternative syntax that is maybe more suitable?

This plugin follows the CSS specification and url fragments on imports do not have any effect there. So the current implementation is actually correct.

@sapphi-red
Copy link
Author

Like this plugin supports, Vite supports importing from node_modules in the import of CSS files.

/* can consume `node_modules`, `web_modules` or local modules */
@import "cssrecipes-defaults"; /* == @import "../node_modules/cssrecipes-defaults/index.css"; */
@import "normalize.css"; /* == @import "../node_modules/normalize.css/normalize.css"; */

We pass a custom resolve option to this plugin to also support Subpath imports (and other resolving features) in CSS. The advantage of supporting this in CSS is that it is intuitive by being consistent with Node.js and allows configuration in one place (package.json).

@romainmenke
Copy link
Collaborator

Those import statements do not have a url fragment.
Can you provide a more complete example please :)


The current implementation completely skips imports with a fragment.
We can make a few changes which might resolve this issue:

  • urls with fragments are no longer skipped
  • urls with fragments are passed to custom resolvers
  • the build-in resolver trims fragments

@sapphi-red
Copy link
Author

sapphi-red commented Feb 20, 2024

Ah, here's a complete example.
https://stackblitz.com/edit/vitejs-vite-1jycbv?file=package.json,style.css,styles%2Fglobal.css&terminal=dev
style.css includes @import '#styles/global.css';. This import is resolved by the custom resolve option passed by Vite using the imports field in the package.json:

  "imports": {
    "#*": "./*"
  },

In this case, #styles/global.css is resolved to <root>/styles/global.css.
It is worth noting that subpath imports always begin with #.

(Please let me know if there's still missing information.)

@romainmenke
Copy link
Collaborator

This is purely non-standard magic right?

And is it correct that this won't work?

  • in css: @import '@org/pkg#sub-path'
  • in package.json of @org/pkg: { "imports": { "sub-path": "..." } }

@RyanZim Do you have a preference?

  1. We can revert to the old behavior and remove any special handling for url fragments.
  2. The current check for fragments is located in a place where we can safely skip import statements. We can move it to the resolver and file loader stage but then we can no longer ignore the statement because of the shape of the surrounding code.
    2.1. We need to trim the fragment and try to load the result of that
    2.2. We throw a hard error.

I would prefer 2.1. but I am also fine with 1.

@sapphi-red
Copy link
Author

This is purely non-standard magic right?

Yes, it's not standardized. It's a feature of Node.js and Vite applied it to CSS.

And is it correct that this won't work?

Yes, that won't work.


Both of those way would work for Vite. Another way I can think of is to default filter option to be isProcessableURL.

@romainmenke
Copy link
Collaborator

After some more thought I think we need to just revert to the old behavior.
Not only Vite, but also other stacks might depend on this same feature.

The intention behind #537 was to make this plugin more correct. Not to make it less useful.

@romainmenke
Copy link
Collaborator

@sapphi-red Can you check if the test scenario I created covers the Vite use case?
https://github.com/postcss/postcss-import/pull/561/files

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

No branches or pull requests

2 participants