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

Unexpected import.meta.url value when bundling #6344

Closed
jsejcksn opened this issue Jun 17, 2020 · 24 comments
Closed

Unexpected import.meta.url value when bundling #6344

jsejcksn opened this issue Jun 17, 2020 · 24 comments

Comments

@jsejcksn
Copy link
Contributor

I've noticed that when using paths relative to import.meta.url, the bundle hardcodes the path. Here is an example:

// relative.ts

import * as path from 'https://deno.land/std/path/mod.ts';

const main = async () => {
  const moduleDir = path.dirname(path.fromFileUrl(import.meta.url));
  const relativeDataPath = path.join(moduleDir, 'file.ext');
  console.log(relativeDataPath);
};

if (import.meta.main) main();
subdirectory % deno run relative.ts 
/path/to/example/subdirectory/file.ext

# output ok, now bundle...

subdirectory % deno bundle relative.ts relative.bundle.js
Bundling file:///path/to/example/subdirectory/relative.ts
Emitting bundle to "relative.bundle.js"
106.77 KB emitted.

# run bundle...

subdirectory % deno run relative.bundle.js 
/path/to/example/subdirectory/file.ext

# output ok, now move bundle to a different directory...

subdirectory % mv relative.bundle.js ../

# change to that directory...

subdirectory % cd ..

# run bundle again...

example % deno run relative.bundle.js 
/path/to/example/subdirectory/file.ext

# output is wrong!

Is this the expected behavior, or is it a bug? If expected, how do I accommodate for this?

@kitsonk
Copy link
Contributor

kitsonk commented Jun 17, 2020

The bundle needs to hardcode the path, as they are no longer independent modules.

What would you expect the behaviour to be, considering it needs to be unique for each module that is part of the bundle.

@jsejcksn
Copy link
Contributor Author

@kitsonk I expected for import.meta.url to be the URL pointing to the bundle when run.

@kitsonk
Copy link
Contributor

kitsonk commented Jun 17, 2020

That assumption might work in certain situations, but others might want to preserve the url at bundle time so each module has its own url.

Looking at Rollup, it is configurable to preserve it or to reflect the runtime environment.

My feeling is that if the module is a local module, it should have a path based on the runtime import.meta.url and its relative path to main module. For example if at bundle time it was ./mod/mod1.ts and at runtime it is being invoked from /foo/bar/app/bundle.js it would become /foo/bar/app/mod/mod1.ts. If it was a remote URL, its URL would just be preserved. That way the behaviour would effectively reflect as closely as possible as if the code wasn't bundled.

@jsejcksn
Copy link
Contributor Author

I think that wouldn't be possible for some cases. Example: ../../../mod1.ts at bundle time and invoked from /dir/bundle.js

Is there something equivalent to entrypoint.meta.url?

@nayeemrmn
Copy link
Collaborator

nayeemrmn commented Jun 17, 2020

@jsejcksn Deno.mainModule (--unstable)

@jsejcksn
Copy link
Contributor Author

@jsejcksn jsejcksn changed the title bundles output hardcoded values for paths that are relative to import.meta.url Unexpected import.meta.url value when bundling Jun 17, 2020
@bartlomieju
Copy link
Member

@jsejcksn does Deno.mainModule() solve this issue for you? Can it be closed?

@jsejcksn
Copy link
Contributor Author

@bartlomieju No.

  1. Deno.mainModule() is not a stable API.

  2. See previous comment:

I think that wouldn't be possible for some cases. Example: ../../../mod1.ts at bundle time and invoked from /dir/bundle.js

@nayeemrmn
Copy link
Collaborator

@jsejcksn I'm a bit confused. Deno.mainModule is not replaced by a hardcoded path upon bundling and gives the location of the bundle when called from the bundle. Was that not what you were looking for?

(Also, we don't keep issues open for stabilisation. It's not helpful for tracking.)

@jsejcksn
Copy link
Contributor Author

@nayeemrmn

Also, we don't keep issues open for stabilisation. It's not helpful for tracking.

Thanks for informing, and I understand.

Are you saying "there's simply not a stable way to get a relative path to the main module at runtime"?

@kitsonk
Copy link
Contributor

kitsonk commented Jul 14, 2020

There's simply a lot of not stable ways to do things, until those APIs are stabilised.

@jsejcksn
Copy link
Contributor Author

@kitsonk Right. I could use Deno.run for everything, etc.

I know that things are evolving, but how do I track that specific problem if the issue is closed?

@nayeemrmn
Copy link
Collaborator

I know that things are evolving, but how do I track that specific problem if the issue is closed?

Release notes. The issue tracker is for maintainer convenience.

@kitsonk
Copy link
Contributor

kitsonk commented Jul 14, 2020

If we feel that there are no questionable semantics about how Deno.mainModule() works then an issue to stabilise Deno.mainModule() would the logical thing to do. It was post 1.0 functionality, so by default it was placed into unstable, just to make sure that it was behaving correctly. Because it doesn't break any existing code, there should be no barrier to stabilising it before 2.0.

But it is better to keep the issues opened focused on specifically what needs to be done.

@jsejcksn
Copy link
Contributor Author

Release notes.

@nayeemrmn I disagree. Parsing every release notes content for keywords related to an issue is not a method of tracking.

@kitsonk:

it is better to keep the issues opened focused on specifically what needs to be done

I certainly understand using GitHub Issues in this specific capacity—it seems maximally helpful to contributors and maintainers. However, this is my experience with a lot of software: there is no afforded mechanism for problem reporters to actually track progress related to the reported problems. I didn't intend for this to fork into a meta discussion about how Issues are used. If further discussion about this topic is desired, is there a suitable place for it?

an issue to stabilise Deno.mainModule() would the logical thing to do

Understood.

Back to the original topic: before closing, I'm wondering if it's possible to expose the rollup option you mentioned for relative/absolute path references to each bundled module's import.meta.url when using deno bundle? This was surprising behavior to me, and I don't think I'll be the last one.

@kitsonk
Copy link
Contributor

kitsonk commented Jul 15, 2020

I'm wondering if it's possible to expose the rollup option you mentioned for relative/absolute path references to each bundled module's import.meta.url when using deno bundle? This was surprising behavior to me, and I don't think I'll be the last one.

But as you mention, that becomes a challenge when a local module is outside of the root of the main module. Given your specific use case Deno.mainModule() is a better solution. The only potential improvement is to potentially add a import.meta.bundle so that at runtime code could better determine what sort of environment it was in. Really want to try to avoid "configuration" for deno bundle as much as possible, especially something that impacts the runtime information/behaviour as it becomes a barrier to that being consumed down the road, because the consumer has to "know" how the bundle was configured. Things like import-maps get fully resolved and don't impact the downstream behaviour of the code.

@jsejcksn
Copy link
Contributor Author

@kitsonk Regarding import.meta.bundle: Would you like for me to open a new issue on that topic, or continue it here? If the former, you can go ahead and close this issue.

@kitsonk
Copy link
Contributor

kitsonk commented Jul 15, 2020

No, sounds logical that we would keep it here. I've added it to #4549.

@nayeemrmn
Copy link
Collaborator

@jsejcksn Can you edit the title of this issue be about the import.meta.bundle boolean?

If it's still contended, the current behaviour of import.meta.url in bundles is the only appropriate one. Preserving it to point to the bundle URL is equivalent to letting every reference to import.meta.main be true, which will completely break it in every case. It's entirely consistent for bundling to just inline any import related construct.

@jsejcksn
Copy link
Contributor Author

Can you edit the title of this issue be about the import.meta.bundle boolean?

Ok; what title would you like?

@kitsonk
Copy link
Contributor

kitsonk commented Nov 5, 2020

As of Deno 1.5 the behaviour of import.meta.url in bundle is to be hardcoded to the original import.meta.url of that module and import.meta.main to be hardcoded to false except for references to import.meta.main which are preserved as import.meta.main.

There is the issue of #8088 which raises issues with that behaviour and will be addressed in future versions of Deno.

Any other suggestions or thoughts about the behaviour should be added to #8088.

@kitsonk kitsonk closed this as completed Nov 5, 2020
@jsejcksn
Copy link
Contributor Author

jsejcksn commented Nov 5, 2020

and import.meta.main to be hardcoded to false except for references to import.meta.main which are preserved as import.meta.main

@kitsonk I'm not sure I understand. When is import.meta.main preserved and when is it hardcoded to false? Can you link me to example code which illustrates the differences?

@kitsonk
Copy link
Contributor

kitsonk commented Nov 5, 2020

The output of the unit test here demonstrates it: https://github.com/denoland/deno/blob/master/cli/tests/bundle/fixture06.out. When the module is a root/entry module import.meta.main is preserved, all other modules when they are flattened in the bundle it is replaced with false.

@jsejcksn
Copy link
Contributor Author

jsejcksn commented Nov 5, 2020

@kitsonk Thanks, that was very clear.

lambdalisue added a commit to vim-denops/denops.vim that referenced this issue Oct 22, 2021
It seems `import.meta.url` is HARDCODED after `deno bundle`

denoland/deno#6344
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

4 participants