@web/rollup-plugin-import-meta-assets fails if encountering any reference to a directory

[undecaf]

Observed behavior

If any module contains a pattern that references a directory, such as

new URL('./', import.meta.url)

then this plugin fails with the message EISDIR: illegal operation on a directory, read. Setting warnOnError: true does not help: if the offending module also contains a pattern that references a file then that file will not be added to the rollup pipeline.

Expected behavior

Patterns that reference directories should be silently ignored by default.

Edit:

My apologies, I just noticed that -- as opposed to what I wrote above -- setting warnOnError: true does help, and other file references are added to the rollup pipeline.
Nevertheless, IMHO if a pattern references a directory then this was probably intended and should not be considered as an error, not even as a not-fatal one.

[bashmish]

Patterns that reference directories should be silently ignored by default.

Why should they be ignored and not resolved to a correct dir path?

I'm thinking of a use-case where one might resolve a dir once and then construct paths to images within this dir.

Am I missing smth in regard to how directories are different in the path resolution algorithm?

[undecaf]

If patterns can be resolved also to a dir path then that would be even better considering the use case that you mentioned (currently such patterns produce an error/a warning but are not resolved).

I think it depends on how one sees the scope of the plugin. When I made the pull request, I had a narrower scope in mind: if there is an asset then the plugin should bundle it, otherwise do nothing. That was my impression when reading the docs.

OTOH, one could think of a wider scope such as:

• if there is a dir or file path then resolve it
• if it is a file path then consider that file an asset and bundle it
• if the dir/file does not exist at build time then raise an error/a warning

From my point of view, both options are agreeable, but the second one is more appealing. Please let me know how you would like to proceed -- thanks!

[undecaf]

@bashmish Sorry, my fingers were faster than my brain...

On second thought, resolving a dir path does not make sense; what should it be resolved to? There is no corresponding asset. So please disregard the second option I mentioned above.

[bashmish]

resolving a dir path does not make sense

@undecaf do you mean it doesn't make sense in the context of assets or in general?

I agree that the scope of this plugin should focus on assets only. But I struggle to understand what the use case for new URL('./', import.meta.url) is? Is it not for assets? Can you explain why you have it in your code?

[undecaf]

do you mean it doesn't make sense in the context of assets or in general?

@bashmish In the context of assets (i.e. like new URL('./img/picture.jpg', import.meta.url)) it should of course be resolved to the path of that file in the bundle.

But resolving resolve something like e.g. new URL('./', import.meta.url) (i.e. replacing it with the path at which the module will be located after bundling) IMHO is not necessary; at runtime this statement will return that path anyway. The same holds true for relative subpaths such as new URL('./img/', import.meta.url).

[undecaf]

But I struggle to understand what the use case for new URL('./', import.meta.url) is? Is it not for assets? Can you explain why you have it in your code?

@bashmish new URL('./', import.meta.url) appears in the code that Emscripten generates for loading WebAssembly files. It is used to locate the WebAssembly file at runtime. As I mentioned above, when bundled this works without having been resolved.

[bashmish]

Thanks for explaining the use-case, it makes it much more clear.

I think it's not about directories though. The tools might include other paths that need to be resolved in runtime only.
Instead of ignoring all directory paths I'd say you should pin-point the ones that can be ignored.

Can you use exclude option to disable the plugin for a specific path where this code appears?
https://modern-web.dev/docs/building/rollup-plugin-import-meta-assets/#exclude

If not, maybe we need another config to exclude or ignore specific files/paths/urls.

[undecaf]

Instead of ignoring all directory paths I'd say you should pin-point the ones that can be ignored.

If warnOnError is true, the current version of the plugin already ignores all paths that reference a directory, i.e. they appear in the bundle as-is. If warnOnError is false then the build fails if a directory path is found.

Therefore my pull request does not change the bundling result compared to what the current plugin version produces; it just silences the directory warning (if warnOnError is true) and lets the build succeed (if warnOnError is false).

Can you use exclude option to disable the plugin for a specific path where this code appears?

I am the author of a package which contains statements like new URL('./', import.meta.url) (referencing a directory) but also new URL('./zbar.wasm', import.meta.url) (referencing an asset) in the same module. Therefore exclude would also prevent the asset from being bundled.

Besides, as that package is a library, all consumers of that package would have to configure the Rollup plugin with exclude. I would prefer to not impose any particular configuration option on the consumers of the package. BTW not having to set warnOnError: true as package consumer was the reason why I submitted this pull request.

[bashmish]

I am the author of a package which contains statements like new URL('./', import.meta.url) (referencing a directory) but also new URL('./zbar.wasm', import.meta.url) (referencing an asset) in the same module. Therefore exclude would also prevent the asset from being bundled.

Then indeed current exclude API won't work.

I myself ran into similar difficulties with another plugin @web/rollup-plugin-html (https://github.com/bashmish/storybook-builder-wds/blob/0.1.0/src/index.ts#L142) when some of the assets could't not be resolved in the index.html, because they were supposed to be served by proxy later. I used extractAssets: false, which worked in my case, although I had to change some code.

This is another plugin and another use-case, but very similar. After that on the back of my head I remembered that we might need to add more fine-grained control over specific assets that we are processing. That's why I was hesitating to make assumptions like "all directory paths should be ignored".

But I also checked the file https://github.com/modernweb-dev/web/blob/master/packages/rollup-plugin-import-meta-assets/src/rollup-plugin-import-meta-assets.js and see that we say:

Checks if a AST node represents this kind of expression: new URL('./path/to/asset.ext', import.meta.url)

And in docs (https://github.com/modernweb-dev/web/blob/master/docs/docs/building/rollup-plugin-import-meta-assets.md) we say:

Rollup plugin that detects assets references relative to modules using patterns such as new URL('./assets/my-img.png', import.meta.url).

Which makes me think that paths without extensions should be skipped, which effectively means ignoring directories (although a path without an extension can be a file too).

I'll review your PR now.

[undecaf]

Which makes me think that paths without extensions should be skipped, which effectively means ignoring directories (although a path without an extension can be a file too).

And a path with an extension could also be a directory. IMHO one has to look into the file system, and then

• if the path represents a file => emit that file as an asset
• if the path represents a directory => ignore it
• if nothing exists at the path => log an error or warning, depending on warnOnError

My pull request makes the plugin behave like that.