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

Add import.meta.resolve() #5572

Merged
merged 1 commit into from
Jun 30, 2022
Merged

Add import.meta.resolve() #5572

merged 1 commit into from
Jun 30, 2022

Conversation

domenic
Copy link
Member

@domenic domenic commented May 21, 2020

Closes #3871.

This introduces a new function, import.meta.resolve(moduleSpecifier), which returns the URL to which a given module specifier would resolve in the context of the current script. That is, it returns the URL that would be imported if you did import(moduleSpecifier).

It will throw an exception if an invalid specifier is given (including one not mapped to anywhere by an import map).

The main use case for this is to get URLs relative to the current script, or relative to a current page. For example:

// Inside https://example.com/my/deep/path/app.mjs
import.meta.resolve("./foo.txt");
// -> "https://example.com/my/deep/path/foo.txt"

or

import.meta.resolve("somepackage/resource.json")
// -> "https://example.com/node_modules/somepackage/lib/resource.json", based on
// an import map that sends "somepackage/" to "https://example.com/node_modules/somepackage/lib/".

(See WHATWG Working Mode: Changes for more details.)


/infrastructure.html ( diff )
/webappapis.html ( diff )

@domenic domenic added addition/proposal New features or enhancements needs implementer interest Moving the issue forward requires implementers to express interest needs tests Moving the issue forward requires someone to write tests topic: script labels May 21, 2020
@guybedford
Copy link
Contributor

This looks like a good start. A useful feature can be to permit a second argument to import.meta.resolve that allows providing a different baseURL for the resolution function - this lets import.meta.resolve behave both as a contextual resolver and as a more general resolver as well.

I can provide some spec along these lines if you'd be open to it?

@domenic
Copy link
Member Author

domenic commented May 21, 2020

I'd like to get the basic version out there first. Adding more features is a sure way to prevent this one from landing.

Base automatically changed from master to main January 15, 2021 07:57
domenic added a commit to WICG/import-maps that referenced this pull request Oct 14, 2021
These have proven over time to be much less interesting than other future extensions we might spend time on. In particular:

* Give up on fallback support. Closes #76, closes #79, closes #83, closes #84. Also closes #79 by removing all the potential complexity there; we can instead discuss on whatwg/html#3871 and whatwg/html#5572.
* Give up on import: URLs. Closes #71, closes #149.
* Give up on built-in module remapping.
@annevk
Copy link
Member

annevk commented Feb 10, 2022

In principle this sounds reasonable to me. Would also like to hear from @jasnell, @lucacasonato, and @codehag. Do we have some kind of place where we document extensions to import.meta?

@domenic
Copy link
Member Author

domenic commented Feb 10, 2022

@jasnell
Copy link

jasnell commented Feb 10, 2022

Yeah, I'm +1 on this. It'll be quite useful, I think.

@lucacasonato
Copy link
Member

lucacasonato commented Feb 10, 2022

I have no strong opinion (maybe +0.1?). Not opposed, but I know some Node folks want this to be async. We are fine with it being sync (and would want it to be sync).

@codehag
Copy link

codehag commented Feb 11, 2022

I don't see an issue with introducing this. I think this would be useful to test import maps, and i imagine it will be useful in such a world were import maps are the norm?

source Outdated Show resolved Hide resolved
@domenic domenic force-pushed the import-meta-resolve branch from fa07fc6 to f91ff0e Compare February 11, 2022 16:53
@domenic domenic force-pushed the import-meta-resolve branch from f91ff0e to 6cc21d3 Compare February 11, 2022 16:54
@domenic domenic removed the needs implementer interest Moving the issue forward requires implementers to express interest label Feb 11, 2022
@domenic
Copy link
Member Author

domenic commented Feb 11, 2022

Awesome, thanks all for the review. I'll plan to merge this after I'm a bit more confident in the tests; I have some up initially at https://chromium-review.googlesource.com/c/chromium/src/+/3456729 but that CL is messy enough that I don't trust them yet.

@GeoffreyBooth
Copy link

Hey, can we slow this down just a bit? I’m a bit concerned with this being sync, since in Node the resolution step is async (and needs to remain so, or it would break various loaders use cases).

Taking a step back, for the non-bare case isn’t import.meta.resolve("./foo.txt") the same as new URL("./foo.txt", import.meta.url)? What’s the advantage of import.meta.resolve?

And the bare case, import.meta.resolve("somepackage/resource.json"), depends on import maps? Is there any anticipation that in the future the resolution of somepackage might be something that happens dynamically, similar to Node’s loaders? And if so, perhaps import.meta.resolve would need to be an async function so that this future dynamic resolution could take advantage of things like network calls?

@domenic
Copy link
Member Author

domenic commented Feb 11, 2022

I think a 7-month incubation period is slow enough?

It's ok for Node to use a different API than the web here.

This is different than the URL constructor because import maps, indeed. There is no plan to make that async; we never want to introduce network calls into module resolution on the web.

@GeoffreyBooth
Copy link

Put another way, is there some advantage in making this sync? If it’s async then we can maintain compatibility with Node or any platform that wants to allow customization in module resolution (and for that custom resolution to potentially be async).

@annevk
Copy link
Member

annevk commented Jun 7, 2022

Aside: was whatwg/js-hosts dissolved due to the creation of the Winter CG?

I was trying to make sense of the status of this in light of mozilla/standards-positions#647 and it seems like it's roughly at where we left it in February. Node.js can make this shape work, but isn't thrilled about it. Deno doesn't necessarily have a need, but if it's added it should be this shape. And browser folks being somewhat positive given that it matches how we resolve JS module specifiers. Is that accurate?

@jasnell
Copy link

jasnell commented Jun 7, 2022

Aside: was whatwg/js-hosts dissolved due to the creation of the Winter CG?

I believe that is the case, yes.

Node.js can make this shape work, but isn't thrilled about it.

That's accurate.

@GeoffreyBooth
Copy link

Node.js can make this shape work, but isn’t thrilled about it.

To add to this, please read this detailed comment about the downsides of Node making resolve sync. This is the kind of user feedback we will be dealing with if we make this change. These all strike me as very reasonable use cases, though mostly more for command-line programs than browser web apps; and I’m not sure if there will be other ways to solve these problems without an async resolve. nodejs/loaders#83 is an attempt to address this, but as a new API it would be a deviation from a cross-platform standard.

@domenic
Copy link
Member Author

domenic commented Jun 7, 2022

From my perspective, that comment displays some fundamental confusion between the resolve phases (a string operation) and the fetch phases (which includes things like checking for existence, permissions, network requests, etc.)

@GeoffreyBooth
Copy link

From my perspective, that comment displays some fundamental confusion between the resolve phases (a string operation) and the fetch phases (which includes things like checking for existence, permissions, network requests, etc.)

In Node, the resolve “phase” includes disk operations. Going back to CommonJS, require.resolve('foo') will search the hard disk for a package named foo, first in local node_modules, then globally, etc. This shipped over ten years ago. So I don’t think it’s unreasonable for users to think of these things as appropriate to occur within resolve. Deciding what to resolve to takes disk operations in Node; it’s not just a string operation. ESM resolution behaves largely the same way, though without automatic extension resolution.

chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Jun 17, 2022
See whatwg/html#5572

Bug: 1296665
Change-Id: I63938700518941d0f65a2a1c7fd13910bd095261
@davidje13
Copy link

From my perspective, that comment displays some fundamental confusion between the resolve phases (a string operation) and the fetch phases (which includes things like checking for existence, permissions, network requests, etc.)

Hi @domenic — sorry for this being quite a delayed reply (I wasn't aware of this thread which linked to my other comment)

I'd like to clarify the use-case which led me to my other comment, which should help explain the bits which you think of as confusion on my part:

In NodeJS's current system, these imports could all resolve to the same file:

import 'bar';
import 'bar/index'; // or something else depending on bar's package.json
import './node_modules/bar/index.js';

And import 'bar'; from a sibling module might be referencing the same bar, or a different bar, depending on the file layout (specifically: which node_modules folder(s) bar lives in).

It's important to know all that before loading the module, because if they resolve to the same thing, we need to use the same instance of that module (e.g. so that any user-land global state is truly global). Once we get to the point of actually loading the module, it's too late. So we need resolve to detect that these are all the same thing and return one consistent URI for them (e.g. file:///wherever/node_modules/bar/index.js).

Now you may say that's a problem with NodeJS's import being too permissive in the specifiers it accepts, but the ability to use named imports is quite important for sharing dependencies while avoiding version conflicts. It's also pretty well established and shared with highly popular tools like WebPack and Rollup, which makes it very popular with destined-for-browser code (pre-bundling).

Import maps have of course been created to fill this need in the browser (with the intent being that all this information which was previously gathered by polling the filesystem is now available declaratively in-advance), but they are at an early stage (and only implemented in one runtime), so I for one cannot yet rely on them for this use-case (even in an experimental capacity).

I could also imagine a future need to lazy-load or even dynamically-generate import maps, especially in local environments like Node where the number of entries could be vast. Even in-browser I could imagine the need to fetch (sub) import maps on-demand once nesting / combining of maps gets more specced out, which would make the resolve call into something more interesting.


My preference (as a userspace developer with a vested interest in keeping parity between browsers and NodeJS) is that import.meta.resolve be defined as asynchronous. I understand that making it asynchronous has the downside that it will not be usable in synchronous contexts, but since asynchronous APIs are already quite common (and in particular used by import()), this doesn't seem likely to cause any problems. If it really truly must not be asynchronous, then it should be unavailable to userspace code until import maps are more mature and widely used, because making it synchronous now will cause problems if it turns out that module resolution is not always going to be a simple string operation (even in-browser).

@lucacasonato
Copy link
Member

FYI, the compartments proposal at TC39 (https://github.com/tc39/proposal-compartments) also defines specifier resolving as being a synchronous operation. (grep for resolveHook).

@domenic domenic removed the needs tests Moving the issue forward requires someone to write tests label Jun 30, 2022
@domenic domenic merged commit b235571 into main Jun 30, 2022
@domenic domenic deleted the import-meta-resolve branch June 30, 2022 03:52
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Jun 30, 2022
See whatwg/html#5572.

Intent to Ship: https://groups.google.com/a/chromium.org/g/blink-dev/c/ZVODFsnIf74

Bug: 1296665
Change-Id: I63938700518941d0f65a2a1c7fd13910bd095261
@GeoffreyBooth

This comment was marked as off-topic.

@domenic

This comment was marked as off-topic.

chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Jul 7, 2022
See whatwg/html#5572.

Intent to Ship: https://groups.google.com/a/chromium.org/g/blink-dev/c/ZVODFsnIf74

Fixed: 1296665
Change-Id: I63938700518941d0f65a2a1c7fd13910bd095261
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Jul 7, 2022
See whatwg/html#5572.

Intent to Ship: https://groups.google.com/a/chromium.org/g/blink-dev/c/ZVODFsnIf74

Fixed: 1296665
Change-Id: I63938700518941d0f65a2a1c7fd13910bd095261
aarongable pushed a commit to chromium/chromium that referenced this pull request Jul 7, 2022
See whatwg/html#5572.

Intent to Ship: https://groups.google.com/a/chromium.org/g/blink-dev/c/ZVODFsnIf74

Fixed: 1296665
Change-Id: I63938700518941d0f65a2a1c7fd13910bd095261
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3456729
Reviewed-by: Kouhei Ueno <[email protected]>
Reviewed-by: Hiroshige Hayashizaki <[email protected]>
Reviewed-by: Yuki Shiino <[email protected]>
Commit-Queue: Domenic Denicola <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1021529}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Jul 7, 2022
See whatwg/html#5572.

Intent to Ship: https://groups.google.com/a/chromium.org/g/blink-dev/c/ZVODFsnIf74

Fixed: 1296665
Change-Id: I63938700518941d0f65a2a1c7fd13910bd095261
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3456729
Reviewed-by: Kouhei Ueno <[email protected]>
Reviewed-by: Hiroshige Hayashizaki <[email protected]>
Reviewed-by: Yuki Shiino <[email protected]>
Commit-Queue: Domenic Denicola <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1021529}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Jul 7, 2022
See whatwg/html#5572.

Intent to Ship: https://groups.google.com/a/chromium.org/g/blink-dev/c/ZVODFsnIf74

Fixed: 1296665
Change-Id: I63938700518941d0f65a2a1c7fd13910bd095261
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3456729
Reviewed-by: Kouhei Ueno <[email protected]>
Reviewed-by: Hiroshige Hayashizaki <[email protected]>
Reviewed-by: Yuki Shiino <[email protected]>
Commit-Queue: Domenic Denicola <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1021529}
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Jul 14, 2022
Automatic update from web-platform-tests
Implement import.meta.resolve()

See whatwg/html#5572.

Intent to Ship: https://groups.google.com/a/chromium.org/g/blink-dev/c/ZVODFsnIf74

Fixed: 1296665
Change-Id: I63938700518941d0f65a2a1c7fd13910bd095261
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3456729
Reviewed-by: Kouhei Ueno <[email protected]>
Reviewed-by: Hiroshige Hayashizaki <[email protected]>
Reviewed-by: Yuki Shiino <[email protected]>
Commit-Queue: Domenic Denicola <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1021529}

--

wpt-commits: edcc51d740e71ace04d1fe1a4ae107ea3e640c2a
wpt-pr: 34188
aosmond pushed a commit to aosmond/gecko that referenced this pull request Jul 15, 2022
Automatic update from web-platform-tests
Implement import.meta.resolve()

See whatwg/html#5572.

Intent to Ship: https://groups.google.com/a/chromium.org/g/blink-dev/c/ZVODFsnIf74

Fixed: 1296665
Change-Id: I63938700518941d0f65a2a1c7fd13910bd095261
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3456729
Reviewed-by: Kouhei Ueno <[email protected]>
Reviewed-by: Hiroshige Hayashizaki <[email protected]>
Reviewed-by: Yuki Shiino <[email protected]>
Commit-Queue: Domenic Denicola <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1021529}

--

wpt-commits: edcc51d740e71ace04d1fe1a4ae107ea3e640c2a
wpt-pr: 34188
mjfroman pushed a commit to mjfroman/moz-libwebrtc-third-party that referenced this pull request Oct 14, 2022
See whatwg/html#5572.

Intent to Ship: https://groups.google.com/a/chromium.org/g/blink-dev/c/ZVODFsnIf74

Fixed: 1296665
Change-Id: I63938700518941d0f65a2a1c7fd13910bd095261
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3456729
Reviewed-by: Kouhei Ueno <[email protected]>
Reviewed-by: Hiroshige Hayashizaki <[email protected]>
Reviewed-by: Yuki Shiino <[email protected]>
Commit-Queue: Domenic Denicola <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1021529}
NOKEYCHECK=True
GitOrigin-RevId: b2f7928219b78bf917e8ae1355d3b43f3ca85014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
addition/proposal New features or enhancements topic: script
Development

Successfully merging this pull request may close these issues.

Proposal: import.meta.resolve (was: import.meta.resolveURL)
10 participants