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

[esbuild-plugin-meta-url] has no effect #47

Closed
maxammann opened this issue Apr 26, 2022 · 5 comments
Closed

[esbuild-plugin-meta-url] has no effect #47

maxammann opened this issue Apr 26, 2022 · 5 comments

Comments

@maxammann
Copy link

I recently started a project which tries to implement a map renderer in Rust. I'm currently evaluating which bundler is the best for the task as I require two features:

  • Inlining webworkers (or any module)
  • Resolving/Handling import.meta.url in CJS and ESM modules, especially for .wasm files.

I tried the plugin @chialab/esbuild-plugin-meta-url to solve the second goal, but esbuild is not copying assets to the output directory.

Config:

build.mjs

import esbuild from 'esbuild';
import metaUrlPlugin from '@chialab/esbuild-plugin-meta-url';

await esbuild.build({
    entryPoints: ['src/index.ts'],
    bundle: true,
    format: "esm",
    platform: "browser",
    outdir: "dist/esbuild/",
    plugins: [
        metaUrlPlugin(),
    ],
});

Is there more configuration needed?

@maxammann
Copy link
Author

The following resolve is returning an empty string within the plugin:

                        const { path: resolvedPath } = await build.resolve(value.split('?')[0], {
                            kind: 'dynamic-import',
                            importer: args.path,
                            namespace: 'file',
                            resolveDir: path.dirname(args.path),
                            pluginData: null,
                        });

@maxammann
Copy link
Author

maxammann commented Apr 26, 2022

I found the issue 🎉

The following works:

new URL('./index_bg.wasm', import.meta.url);

while this is not working:

new URL('index_bg.wasm', import.meta.url);

Both should work afaik.

Should we just prepend the ./ when resolving the file through esbuild?

@edoardocavazza
Copy link
Member

Hello! Thanks for opening the issue and the PR.

I am afraid this is the intended behavior of esbuild. Under the hoods, we use the esbuild resolution algorithm to resolve paths:

If you want to resolve a file name in a specific directory, make sure the input path starts with ./. Otherwise the input path will be treated as a package path instead of a relative path. This behavior is identical to esbuild's normal path resolution logic.

We cannot automatically add ./ urls since we want to maintain the ability to import NPM modules as well as match onResolve callbacks from other plugins/configurations.

BUT we could add a check for relative urls if the first try fails and it is not a relative url already. Would you like to update the PR with this behavior?

@maxammann
Copy link
Author

I kind of agree that resolving modules could be handy for users. I would put that under a feature flag though as it does not conform to the default behavior.

In plain ES6 new URL("test.js", import.meta.url), called from https://example.com/main.js will resolve to: https://example.com/test.js

That means by definition resolving a file though URL + import.meta.url is relative. When transforming such a URL construct we should keep the relativity.

I tried the current implementation against the generated code by wasm_bindgen which expects it to be foremost relative.

@edoardocavazza
Copy link
Member

Hello @maxammann. We have released a new version of the plugin (0.15.17). As you suggested, we tried to closely match the standard URL behavior, so we started to prefer local imports over modules resolution. We still support it for backward compatibility, but now it raises a warning. Thank you!

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

Successfully merging a pull request may close this issue.

2 participants