- Notifications
You must be signed in to change notification settings - Fork2.9k
Add import.meta.url#3141
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Add import.meta.url#3141
Uh oh!
There was an error while loading.Please reload this page.
Conversation
This integrates with the stage 3 import.meta proposal located athttps://github.com/tc39/proposal-import-meta/. This is based onhttps://github.com/tc39/proposal-import-meta/blob/f5d39bc471a5bf2791708f9a3fec943380d9e3ee/HTML%20Integration.mdalthough it only includes the easier part, import.meta.url.import.meta.scriptElement is still being discussed, at#1013, and assuch is excluded for now.
lgtm as an implementer. Drafted Chromium implementation based on the proposed spec inhttps://chromium-review.googlesource.com/c/chromium/src/+/726525. I'm also adding wpt tests there. |
It occurs to me this could benefit from a bit of web-developer facing documentation, e.g. a domintro box. I'll try to tack that on. |
I've filed#3208 to separately work on developer-facing documentation. Tests have landed inhttps://github.com/w3c/web-platform-tests/tree/master/html/semantics/scripting-1/the-script-element/module/import-meta Can I get editorial review from one of the editors, and let's merge this? |
Okay, looking at the issue it seems this has support from Chromium and WebKit at least, as well as web developers. |
<h6 | ||
id="hostgetimportmetaproperties"><dfn>HostGetImportMetaProperties</dfn>(<var>moduleRecord</var>)</h6> | ||
<p>The <cite>import.meta</cite> proposal contains an implementation-defined <span |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Would it not be host-defined given the Host prefix?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Other nearby sections went with implementation-defined; host feels like JS-spec-specific jargon to me?
<li><p>Let <var>urlString</var> be <var>module script</var>'s <span | ||
data-x="concept-script-base-url">base URL</span>, <span | ||
data-x="concept-url-serializer">serialized</span>.</p></li> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Wait why the base URL? That could easily end up being the same for a bunch of scripts. Wouldn't we want the resource URL here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Every script has abase URL which is the URL it was fetched from, for external scripts, or the URL of the page it was embedded in, for inline scripts. It's called the base URL since it's used to resolve relative imports.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
We should rename it then, but seems fine as follow-up. It's fine if a resource doesn't have a base URL other than its own URL, but we should at least have a term for its own URL.
https://bugs.webkit.org/show_bug.cgi?id=178672Reviewed by Darin Adler.Source/WebCore:This patch implements `import.meta.url` field, which holds a base urlof the currently executing module[1].In the case of embedded modules, this field becomes the URL of theembedding HTML file. In the case of imported modules, the URL becomesthe URL of the executed module script file.[1]:whatwg/html#3141* bindings/js/JSDOMWindowBase.cpp:(WebCore::JSDOMWindowBase::moduleLoaderCreateImportMetaProperties):* bindings/js/JSDOMWindowBase.h:* bindings/js/ScriptModuleLoader.cpp:(WebCore::ScriptModuleLoader::moduleURL):(WebCore::ScriptModuleLoader::evaluate):(WebCore::ScriptModuleLoader::createImportMetaProperties):* bindings/js/ScriptModuleLoader.h:LayoutTests:* js/dom/modules/import-meta-url-expected.txt: Added.* js/dom/modules/import-meta-url.html: Added.* js/dom/modules/script-tests/import-meta-url-second-level.js: Added.* js/dom/modules/script-tests/import-meta-url-top-level.js: Added.* js/dom/modules/script-tests/import-meta-url.js: Added.git-svn-id:http://svn.webkit.org/repository/webkit/trunk@224736 268f45cc-cd09-0410-ab3c-d52691b4dbfc
https://bugs.webkit.org/show_bug.cgi?id=178672Reviewed by Darin Adler.Source/WebCore:This patch implements `import.meta.url` field, which holds a base urlof the currently executing module[1].In the case of embedded modules, this field becomes the URL of theembedding HTML file. In the case of imported modules, the URL becomesthe URL of the executed module script file.[1]:whatwg/html#3141* bindings/js/JSDOMWindowBase.cpp:(WebCore::JSDOMWindowBase::moduleLoaderCreateImportMetaProperties):* bindings/js/JSDOMWindowBase.h:* bindings/js/ScriptModuleLoader.cpp:(WebCore::ScriptModuleLoader::moduleURL):(WebCore::ScriptModuleLoader::evaluate):(WebCore::ScriptModuleLoader::createImportMetaProperties):* bindings/js/ScriptModuleLoader.h:LayoutTests:* js/dom/modules/import-meta-url-expected.txt: Added.* js/dom/modules/import-meta-url.html: Added.* js/dom/modules/script-tests/import-meta-url-second-level.js: Added.* js/dom/modules/script-tests/import-meta-url-top-level.js: Added.* js/dom/modules/script-tests/import-meta-url.js: Added.Canonical link:https://commits.webkit.org/195624@maingit-svn-id:https://svn.webkit.org/repository/webkit/trunk@224736 268f45cc-cd09-0410-ab3c-d52691b4dbfc
This integrates with the stage 3 import.meta proposal located at
https://github.com/tc39/proposal-import-meta/. This is based on
https://github.com/tc39/proposal-import-meta/blob/f5d39bc471a5bf2791708f9a3fec943380d9e3ee/HTML%20Integration.md
although it only includes the easier part, import.meta.url.
import.meta.scriptElement is still being discussed, at#1013, and as
such is excluded for now.
/cc @whatwg/modules
This is ready to merge IMO, but needs tests before we can do so. We'll get some through Chrome's implementation if nothing happens sooner.
Review appreciated, both editorial and from implementers.