Use `rollupOptions.options.assetFileNames` when specified in the Vite config

[lsdsjy]

Typically in vite we use build.rollupOptions.output.assetFileNames to tweak output directory and file names, like:

            rollupOptions: {
                output: {
                    assetFileNames: (asset) => {
                        if (
                            ['.jpg', '.png', '.svg', '.avif', '.webp'].some((ext) =>
                                asset.name?.endsWith(ext),
                            )
                        ) {
                            return 'images/[name]-[hash][extname]';
                        }
                        return 'assets/[name]-[hash][extname]';  // for other assets like fonts and audios etc.
                    },
                },

While this plugin provides an assetsDir option, it should still conform to the idiom above when assetsDir is not specified. But unfortunately that's not the current behavior.

I can submit a PR for this if feasible.

[ElMassimo]

Hi there!

In the example, passing assetsDir: 'images' would achieve the expected behavior.

---

Since assetFilenames expects a PreRenderedAsset, supporting it would burden this plugin with having to replicate that Rollup interface for compatibility.

In addition, supporting all of the placeholders ([extname]) is a similar problem, there's always the chance that Rollup adds more in the future, and they would not work as expected if they are not implemented in this plugin.

I'm leaning towards not supporting this, as it's not a "Vite"-level feature, it's an advanced feature targeting Rollup specifically, and it would increase the surface and complexity of this plugin in a way that is not proportional to how often it's used.

[lsdsjy]

In the example, passing assetsDir: 'images' would achieve the expected behavior.

Yes, it's also the workaround I'm using. But assetsDir can't support patterns like this: assets/image-[name][hash][extname].

Since assetFilenames expects a PreRenderedAsset, supporting it would burden this plugin with having to replicate that Rollup interface for compatibility.
In addition, supporting all of the placeholders ([extname]) is a similar problem, there's always the chance that Rollup adds more in the future, and they would not work as expected if they are not implemented in this plugin.

I don't think we need to parse and apply the pattern by ourselves. We can just add the asset into the bundle via this.emitFile during the generateBundle hook, which will leave the name calculating process to Rollup as the Rollup docs say:

If a fileName is provided, it will be used unmodified as the name of the generated file, throwing an error if this causes a conflict. Otherwise if a name is supplied, this will be used as substitution for [name] in the corresponding output.chunkFileNames or output.assetFileNames pattern

So we don't need to calculate fileName at all. Besides, I believe this.emitFile is a more "official" way to add file to bundle according to the docs:

To emit additional files, use the this.emitFile plugin context function.

[ElMassimo]

This plugin provides an api to write images outside of the build lifecycle, so emitFile is not a viable option for the use cases I have.

[lsdsjy]

IIUC the api you mentioned is independent, not used by the vite plugin. I think it's ok that we keep generatedImages as OutputAsset and just ignore the fileName property when consumed in the generateBundle hook of the vite plugin(if assetsDir is not specified).

[ElMassimo]

One of the things that makes this plugin complex is that you need the resolved asset URLs as a string in JS, so that they can be injected into HTML as-is.

Using emitFile in generateBundle would force you to do a second pass replacing any strings that reference the assets, to replace the original filename with the fingerprinted version.

Feel free to submit a PR if you find a way to make it work, but I don't think it a small feature like assets/image-[name][hash][extname] justifies a large change to the architecture of the plugin.