-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
Implement synchronous import.meta.resolve(…)
#2472
Comments
On the server, module resolution has to be async. We don't know where a module might point to until we've visited other files (like package.json). We may even need to download package.json to find out. That operation should not block the server by default. Browsers don't have this problem because browser module resolution is very simple. Bun does implement
Per the ECMAScript spec,
|
That being said, on the bundler side of things Bun can implement the |
Hmm,
|
Just wanted to make a note that If/when this goes to stable next week, this means that all the following will support the same synchronous
I'd like to be able to support However, it seems likely to me that developers and apps in the wider ecosystem will start assuming a synchronous result based on all the other environments. I can also imagine bundlers/minifiers removing the |
FWIW, (I'm still fine with using |
import.meta.resolve(…)
import.meta.resolve(…)
If I may, I'd like to just comment on the usage of We at PeacockTV and SkyShowtime need to support all kinds of TVs, set-top boxes and console, some of which were launched more than 10 years ago (PlayStation 4 is one such example). Some of these are based on older versions of Chromium, and lack I know we could add polyfills but in order to keep our bundle size small we usually try to rewrite our code before adding one. What @Jarred-Sumner said:
Is super important and would allow us to deliver smaller builds to this other class of devices. |
Hello 👋 I’m part of the team responsible for landing sync @guilhermesimoes how do you plan to get |
We already have |
I think that makes sense when writing code that's only meant for If a code author knows about
… but I wouldn't want to make a bet that this will be handled correctly by all browsers2, bundlers, and transpilers3. I also wouldn't be too enthused to make a pull request to every project that uses If there is a need for both sync and async flavors, I'd strongly advocate for Footnotes
|
what i meant was we could use our existing implementation of
This sounds like a good idea to me, most of the work is just swapping the two functions around. There will be a later effort of getting synchronous resolve to work with asynchronous |
Also want to point out that it should be possible to resolve anything that dose not resolve into a module. import.meta.resolve('./' + Math.random()) Deno and browser dose not look up any module when resolving paths. What is the expected behavior? Why is that the expected behavior?i did kind of expect i kind of expect all of the node_modules folder + node core modules to behave more or less import-maps
What do you see instead?One thing that's happening in Bun is:
i think Bun should kind of be creating a import-map.json up front when it install any dependencies NodeJS just fixed this: nodejs/node#49010 |
@paperdave Would you accept a PR for this / if I wanted to tackle this, is there any advice you could give me? The |
we definitely need to align what import.meta.resolve does in node something im still wondering is if node.js is going to make import.meta.resolve return a string like web browsers and deno do, or keep it as a URL object. would appreciate a pr for this. most of the code is already there to do this. |
I hope they keep it as a string to align with what browser are doing. |
🤩
(FWIW,
Alright, I'll see if I can get my |
Hmm, interesting. There actually seems to be a mismatch with the documentation: https://nodejs.org/api/esm.html#importmetaresolvespecifier
EDIT: filed an issue at nodejs/node#49695 |
Node's |
Could you describe the difference? This is what I see in > echo "console.log(process.versions.node); console.log(import.meta.resolve('./rel'));" > /tmp/test.mjs
> node /tmp/test.mjs
20.6.1
URL {
href: 'file:///private/tmp/rel',
origin: 'null',
protocol: 'file:',
username: '',
password: '',
host: '',
hostname: '',
port: '',
pathname: '/private/tmp/rel',
search: '',
searchParams: URLSearchParams {},
hash: ''
}
|
wtf, that is weird, i had to try it out for myself. then i tried your exact version and then i got back a URL instance. like what gives?!?! turns out if the file or the directory exist, then you get back a string. Sooooo inconsistent |
Oh, woah, nice debugging — definitely looks like it! > echo "console.log(process.versions.node); console.log(import.meta.resolve('.'));" > /tmp/test.mjs
> node /tmp/test.mjs
20.6.1
file:///private/tmp/ |
It’s a bug, fixed in nodejs/node#49698. |
What is the problem this feature would solve?
It is common to write apps and library code in JS that references relative file paths, such as:
import.meta.resolve(…)
provides a standard way to refer to a relative resource, and also supports bare names and absolute paths. It is already supported in all major browsers anddeno
. It would be valuable forimport.meta.resolve(…)
to be available in all target environments, includingbun
.What is the feature you are proposing to solve the problem?
Implement https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/import.meta/resolve
What alternatives have you considered?
As a library author, I can use code that uses
new URL(…, import.meta.url).href
instead. However:import.meta.resolve(…)
is more ergonomic and conveys clear intentions compared to being a "magic pattern", andThe text was updated successfully, but these errors were encountered: