fix(css): vite can't handle images in css correctly

[wkstudy]

Description

The scene where the problem occurs during the following demo:
click me

1. Run npm run build, vite prompts that the package is successful but the images in the css are not processed
2. After deleting the postcss plugin of the demo, run npm run build again, the packaging is normal

Additional context

The reason for the problem is

1. Currently vite-url-plugin is executed after the user-defined component

// https://github.com/wkstudy/vite/blob/4194cce60f7d9a5664ef9aea279f1f69631cdc84/packages/vite/src/node/plugins/css.ts#L695

postcssPlugins.push(
    UrlRewritePostcssPlugin({
      replacer: urlReplacer
    }) as Postcss.Plugin
  )

2. vite-url-plugin relies on the input.file property to change the resource path

// https://github.com/wkstudy/vite/blob/4194cce60f7d9a5664ef9aea279f1f69631cdc84/packages/vite/src/node/plugins/css.ts#L865
 const importer = declaration.source?.input.file

3. input.file must have opts to generate, otherwise it will disappear

// https://github.com/postcss/postcss/blob/d533f80b3cb4ef394cc7b523f675828ef7ec8466/lib/parse.js#L8
...
function parse(css, opts) {
  const input = new Input(css, opts)
}

...
// https://github.com/postcss/postcss/blob/d533f80b3cb4ef394cc7b523f675828ef7ec8466/lib/input.js#L36
...
if (opts.from) {
  if (!pathAvailable || /^\w+:\/\//.test(opts.from) || isAbsolute(opts.from)) {
    this.file = opts.from
  } else {
    this.file = resolve(opts.from)
  }
}
...

4. The postcss-px2rem-excludeplugin is called in my demo, and the postcss.parse method is called here, but the opts object is not passed, causing the subsequent css astobj to lose the input.file attribute

// https://github.com/saionjisekai/px2rem-postcss/blob/1edc6763ad2fe8288ac6a8bda482319f006d8bb6/lib/index.js#L15
    result.root = postcss.parse(newCssText)

5. The final input.file value is undefined, the vite package is successful but the url in the css is not processed
6. My understanding is that there is no problem with vite and postcss plugins, but it is best to call the plugin inside vite first, and then call the user-defined plugin

---

What is the purpose of this pull request?

• Bug fix
• New Feature
• Documentation update
• Other

Before submitting the PR, please make sure you do the following

• Read the Contributing Guidelines.
• Read the Pull Request Guidelines and follow the Commit Convention.
• Check that there isn't already a PR that solves the problem the same way to avoid creating a duplicate.
• Provide a description in this PR that addresses what the PR is solving, or reference the issue that it solves (e.g. fixes #123).
• Ideally, include relevant tests that fail without this PR but pass with it.

[jonathantneal]

Just chiming in that the issue seems clear and the fix seems reasonable. Patak mentioned this issue to me, which is why I’m leaving this review.

[wkstudy]

Patak

Just chiming in that the issue seems clear and the fix seems reasonable. Patak mentioned this issue to me, which is why I’m leaving this review.

Thanks, keep learning from you guys

[bluwy]

Shouldn't this be fixed in postcss-px2rem? You've linked https://github.com/saionjisekai/px2rem-postcss/blob/1edc6763ad2fe8288ac6a8bda482319f006d8bb6/lib/index.js#L15 in no4, where the opts isn't passed. But checking the types for .parse

export interface ProcessOptions {
  /**
   * The path of the CSS source file. You should always set `from`,
   * because it is used in source map generation and syntax error messages.
   */
  from?: string

  /**
   * Source map options
   */
  map?: SourceMapOptions | boolean
}

from is encouraged to be set but it's not. I think making this change is breaking too which could affect plugins that alters the url, so I think this should be fixed in userland (if possible).

[patak-dev]

Thanks for doing a review @jonathantneal, @bluwy's comment is in line with what I'm also worried about in this PR. I imagine a plugin injecting a URL or modifying it isn't uncommon, no?

[wkstudy]

Shouldn't this be fixed in postcss-px2rem? You've linked https://github.com/saionjisekai/px2rem-postcss/blob/1edc6763ad2fe8288ac6a8bda482319f006d8bb6/lib/index.js#L15 in no4, where the opts isn't passed. But checking the types for .parse

export interface ProcessOptions {
  /**
   * The path of the CSS source file. You should always set `from`,
   * because it is used in source map generation and syntax error messages.
   */
  from?: string

  /**
   * Source map options
   */
  map?: SourceMapOptions | boolean
}

from is encouraged to be set but it's not. I think making this change is breaking too which could affect plugins that alters the url, so I think this should be fixed in userland (if possible).

it works, thanks a lot. And I have some other ideas

1. In the existing postcss ecosystem, there may be some plugins that will destroy the ast structure of css just like postcss-px2rem, causing the vite-url-plugin plugin that strongly depends on the input.file property to fail. This may be rare. But if something goes wrong, I think it's more difficult to find the reason.
2. As for whether the execution order of vite-url-plugin is changed, will it affect other url plugins? I think there is a certain chance, but even if there is a problem here, it is easier for me to accept it. Because this is a plug-in sequence problem, it is relatively common and the developer knows how to solve it, but the above problem is too hidden, and it gives the developer the feeling that "there is a problem with vite, and my image cannot be loaded", although this is not the case .

All in all, I think that as long as vite can provide developers with a normal postcss structure, users can accept and find a solution even if there is a sequence problem, but if the user's input affects the normal operation of vite, it may not be reasonable.

In the end, this is actually a relatively small issue, thank you for your attention

[bluwy]

@wkstudy Does #8183 help in this case? It should report a warning when there's a postcss plugin that doesn't pass the field.

[wkstudy]

@wkstudy Does #8183 help in this case? It should report a warning when there's a postcss plugin that doesn't pass the field.

Reasonable

[patak-dev]

@wkstudy let's close this PR in favor of #8183. Please get back to us in case postcss-px2rem-exclude decides they want to accept the PR to fix the issue upstream and they give a good reason. @jonathantneal thanks again for checking the issue, if you have more info about the number of PostCSS plugins that may start to provoke the warning in #8183 please let us know. Maybe it is more common than what we're assuming. This solution isn't set in stone, let's see how users react to the warning and we can re-evaluate before releasing v3 stable