The import.meta proposal is at stage 3, and shipping in stable Chrome since Chrome 64. We should aim to ship this when we ship modules on stable at all.

Jon, are you planning to have this ship as part of our initial ES6 modules implementation? The spec is definitely stable enough for us to ship it to release, and it'd be a valuable feature to have from the get-go.

(In reply to Till Schneidereit [:till] PTO February 6 - 16 from comment #1) I wasn't planning on shipping this as part of the initial implementation, but I'd like to work on it straight after. Do you think it's worth making this block the initial implementation?

I agree that this doesn't have to block initial implementation. Which is a good thing, given that you've since flipped the switch to let modules ride the trains :) Given that the contents of import.meta are implementation defined, one reason for getting this in sooner rather than later is to ensure that there are multiple implementations and hence a reason to coordinate what goes into import.meta. I don't know what the Chrome team's plans around this are, but we should ensure that we have a dialog with them about this.

Any updates on when this might be ship wrt basic module support? On the Polymer project we're about to publish 3.0 using native modules that rely on dynamic import() and import.meta.url. In our tools we'd like to just say that module support includes dynamic import() and import.meta.url to reduce the build permutations we have to support. So far Chrome and Safari Technology Preview support both.

We are going to ship modules, without dynamic import() and import.meta. Jon can probably comment on when we plan on implementing those features.

FWIW we have tests at http://w3c-test.org/html/semantics/scripting-1/the-script-element/module/import-meta/import-meta-url.html ready to go.

Patch to implement parsing of |import.meta|. Currently there is no state in the parser to say whether we were parsing for the script goal or the module goal (we currently only need to know whether we are at the top level of a module), so this adds that state and propagates it through full parsing of lazy scripts. Mostly import.meta is handled similarly to new.target. One complication was the statement processing where an 'import' keyword can be the start of an import declaration or an import.meta expression. I added importDeclarationOrImportMeta() methods to handle this. I'm not sure if this is the best way to do it though.

Implement import.meta in the JS shell. This adds a slot to module objects to hold the meta object. We get the module associated with a script by walking the scope chain. There is a single hook for the embedding to perform the role of both the HostGetImportMetaProperties and HostFinalizeImportMeta operations.

JIT support. If we baseline compile a script that uses import.meta we create the meta object at that time for the sake of simplicity. For Ion we assume that this has been done by baseine.

Set up module metadata hook to implmement import.meta for modules in the browser. This just defines the 'url' property. HTML spec is here: https://html.spec.whatwg.org/multipage/webappapis.html#hostgetimportmetaproperties

Comment on attachment 8976589 [details] [diff] [review] bug1427610-2-implement-import-meta-shell Review of attachment 8976589 [details] [diff] [review]: ----------------------------------------------------------------- LGTM! ::: js/src/builtin/ModuleObject.cpp @@ +1645,5 @@ > + > +JSObject* > +js::GetOrCreateModuleMetaObject(JSContext* cx, HandleObject moduleArg) > +{ > + RootedModuleObject module(cx, &moduleArg->as<ModuleObject>()); `Handle<ModuleObject*> module = moduleArg.as<ModuleObject>();` so we don't need to root it twice. @@ +1651,5 @@ > + JSObject* obj = module->metaObject(); > + if (obj) > + return obj; > + > + obj = cx->runtime()->moduleMetaObjectHook(cx, module); This is a bit more permissive than the spec proposal, because it also allows the embedding to return an exotic object (i.e. not a js::PlainObject). This is probably not an issue as long as it isn't misused by the embedder. (If we create the object here and then pass it to the embedding, we could ensure that the object is always in the tenured heap, because maybe it's preferable to have a tenured object when baking it into the jit-code. Please double-check this with jandem for Part 3!) The spec proposal also requires that this hook never returns an abrupt completion. I don't think we can (b/c of OOM) and should have these requirements in our implementation, so handling the case when |obj| is nullptr here seems correct to me. ::: js/src/builtin/ModuleObject.h @@ +253,5 @@ > EnvironmentSlot, > NamespaceSlot, > StatusSlot, > EvaluationErrorSlot, > + MetaObjectSlot, The last slot before regressing bug 1420412! :-) ::: js/src/jit-test/tests/modules/import-meta.js @@ +1,4 @@ > +// |jit-test| module > + > +// import.meta is an object. > +assertEq(typeof import.meta, "object"); Maybe additionally `assertEq(import.meta !== null, true);`, because |typeof null === "object"|. @@ +24,5 @@ > +let otherImportMeta = getOtherMetaObject(); > +assertEq(otherImportMeta.url.endsWith("exportImportMeta.js"), true); > + > +// By default the import.meta object will be extensible, and its properties will > +// be writable, configurable, and enumerable. Maybe add an explicit test for this? ``` assertEq(Object.isExtensible(import.meta), true); var desc = Object.getOwnPropertyDescriptor(import.meta, "url"); assertEq(desc.writable, true); assertEq(desc.enumerable, true); assertEq(desc.configurable, true); assertEq(desc.value, import.meta.url); ``` And also add a test for the prototype? ``` assertEq(Object.getPrototypeOf(import.meta), null); ``` @@ +34,5 @@ > +assertEq(import.meta.newProp, 42); > + > +let found = new Set(); > +for (let p in import.meta) > + found.add(p); Better: `let found = new Set(Reflect.ownKeys(import.meta));` That way symbols and non-enumerable properties are included, too. @@ +45,5 @@ > +delete import.meta.newProp; > + > +found = new Set(); > +for (let p in import.meta) > + found.add(p); Also here: `let found = new Set(Reflect.ownKeys(import.meta));` ::: js/src/shell/js.cpp @@ +4312,5 @@ > > +static JSObject* > +ShellModuleMetaObjectHook(JSContext* cx, HandleObject module) > +{ > + RootedObject obj(cx, JS_NewPlainObject(cx)); |JS_NewObjectWithGivenProto(cx, nullptr, nullptr)| to match the default prototype in the spec proposal ("Set importMeta to ObjectCreate(null)."). @@ +4318,5 @@ > + return nullptr; > + > + // For the shell, just use the script's filename as the base URL. > + RootedScript script(cx, module->as<ModuleObject>().script()); > + const char* filename = script->scriptSource()->filename(); Is |filename| guaranteed to be non-null here? For example when the module is manually constructed using the shell test-API? @@ +4323,5 @@ > + RootedString url(cx, NewStringCopyZ<CanGC>(cx, filename)); > + if (!url) > + return nullptr; > + > + JS_DefineProperty(cx, obj, "url", url, JSPROP_ENUMERATE); if (!JS_DefineProperty(...)) return nullptr; to handle possible OOM. ::: js/src/vm/EnvironmentObject.h @@ +1169,1 @@ > ModuleEnvironmentObject* GetModuleEnvironmentForScript(JSScript* script); Nit: Blank line between both functions. ::: js/src/vm/Interpreter.cpp @@ +4213,5 @@ > END_CASE(JSOP_NEWTARGET) > > +CASE(JSOP_IMPORTMETA) > +{ > + ReservedRooted<JSObject*> module(&rootObject0, GetModuleObjectForScript(script)); Add `MOZ_ASSERT(module);` because GetOrCreateModuleMetaObject(...) expects a non-null argument, but GetModuleObjectForScript(...) has an explicit `return nullptr;`? @@ +4214,5 @@ > > +CASE(JSOP_IMPORTMETA) > +{ > + ReservedRooted<JSObject*> module(&rootObject0, GetModuleObjectForScript(script)); > + JSObject* metaObject = GetOrCreateModuleMetaObject(cx, module); Is it possible to use `module.as<ModuleObject>()` here and then change GetOrCreateModuleMetaObject to only accept `Handle<ModuleObject*>` for better type safety? Or are they some restrictions because of reserved rooted? ::: js/src/vm/Opcodes.h @@ +2348,5 @@ > + /* > + * Push "import.meta" > + * > + * Category: Variables and Scopes > + * Type: Arguments Hmm, I don't think "Arguments" is correct to reuse here. Maybe just add a new "Type: Module"?

Comment on attachment 8976652 [details] [diff] [review] bug1427610-3-jit-support Review of attachment 8976652 [details] [diff] [review]: ----------------------------------------------------------------- Nice.

Thanks for all the comments! I changed the hook so that object is created first and then passed in, which is a bit safer as you touched on in your comments. > Is |filename| guaranteed to be non-null here? For example when the module is manually constructed using the shell test-API? Yes, this ends up being "<string>" in that case, but I added an assert. > Is it possible to use `module.as<ModuleObject>()` here and then change GetOrCreateModuleMetaObject to only accept `Handle<ModuleObject*>` for better type safety? Or are they some restrictions because of reserved rooted? This didn't work because of |module| being a reserved rooted. Other comments addressed.

Comment on attachment 8976937 [details] [diff] [review] bug1427610-2-implement-import-meta-shell Review of attachment 8976937 [details] [diff] [review]: ----------------------------------------------------------------- Thanks, still looks good! ::: js/src/builtin/ModuleObject.cpp @@ +1649,5 @@ > + HandleModuleObject module = moduleArg.as<ModuleObject>(); > + if (JSObject* obj = module->metaObject()) > + return obj; > + > + RootedObject metaObject(cx, JS_NewObjectWithGivenProto(cx, nullptr, nullptr)); Nit: Now that we're allocating the object in the js-engine, we should directly call |NewObjectWithGivenProto<PlainObject>(cx, nullptr);| instead of going through JSAPI. ::: js/src/vm/Runtime.h @@ +941,5 @@ > js::MainThreadData<JS::ModuleResolveHook> moduleResolveHook; > > + // A hook that implements the abstract operations > + // HostGetImportMetaProperties and HostFinalizeImportMeta. > + js::MainThreadData<JS::ModuleMetadataHook> moduleMetadataHook; Do we need to explicitly initialize the field in the constructor? Asking because I just saw that |moduleResolveHook| is explicitly initialized here <https://searchfox.org/mozilla-central/rev/da499aac682d0bbda5829327b60a865cbc491611/js/src/vm/Runtime.cpp#178>.

Updated patch following review feedback above. Sorry for the churn.

Comment on attachment 8976957 [details] [diff] [review] bug1427610-4-script-loader Review of attachment 8976957 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/script/ScriptLoader.cpp @@ +809,5 @@ > + JS::Handle<JSObject*> aMetaObject) > +{ > + JS::Value value = JS::GetModuleHostDefinedField(aModule); > + auto script = static_cast<ModuleScript*>(value.toPrivate()); > + MOZ_ASSERT(script->ModuleRecord() == aModule); Can you make this a MOZ_DIAGNOSTIC_ASSERT() since its a cheap comparison? Also, would it be worth MOZ_DIAGNOSTIC_ASSERT'ing that aModule is not nullptr. @@ +812,5 @@ > + auto script = static_cast<ModuleScript*>(value.toPrivate()); > + MOZ_ASSERT(script->ModuleRecord() == aModule); > + > + nsAutoCString url; > + script->BaseURL()->GetAsciiSpec(url); I think you need to check for BaseURL() being nullptr since its a cycle collected member. It can get unlinked while the ModuleScript is still alive. As a side note, is there any impact to GC/CC from storing the ModuleScript in the metadata hook mechanism? That seems to be defined in another patch and I'm not sure how that impact the lifecycle. Also, if we are going to assume GetAsciiSpec() is infallible, then please wrap the call in MOZ_ALWAYS_SUCCEEDS() to assert that in debug builds.

Comment on attachment 8976171 [details] [diff] [review] bug1427610-1-parse-import-meta Review of attachment 8976171 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/frontend/Parser.cpp @@ +8726,5 @@ > return null(); > lhs = handler.newSuperBase(thisName, pos()); > if (!lhs) > return null(); > + Style nit: no blank line here.

(In reply to Ben Kelly [:bkelly] from comment #16) > As a side note, is there any impact to GC/CC from storing the ModuleScript > in the metadata hook mechanism? That seems to be defined in another patch > and I'm not sure how that impact the lifecycle. The ModuleScript has a pointer to the JS ModuleObject which acts as a strong reference and keeps the ModuleObject alive as long as the ModuleScript is live. The ModuleObject has a pointer back to the ModuleScript set with SetModuleHostDefinedField() which acts like a weak pointer and doesn't keep the ModuleScript alive (we don't increment its ref count when this is set). When the ModuleScript is unlinked we clear this back pointer. So this doesn't influence the lifetime of the ModuleScript. I'll add a diagnostic assert that the back pointer is set. It's not possible to still be running scripts after the ScriptLoader has died, right?

(In reply to Jon Coppeard (:jonco) from comment #18) > I'll add a diagnostic assert that the back pointer is set. On second thoughts, I'll play it safe and report an error if this is not set.

Pushed by jcoppeard@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/277bd9cf9edc Implement import.meta in the JS frontent r=jorendorff https://hg.mozilla.org/integration/mozilla-inbound/rev/a506ea1db794 Implement import.meta in the JS shell r=anba https://hg.mozilla.org/integration/mozilla-inbound/rev/847453f52aab Support import.meta in the JITs r=jandem https://hg.mozilla.org/integration/mozilla-inbound/rev/9a5e8a990f0a Implement import.meta in the browser r=bkelly

Created web-platform-tests PR https://github.com/w3c/web-platform-tests/pull/11117 for changes under testing/web-platform/tests

https://hg.mozilla.org/mozilla-central/rev/277bd9cf9edc https://hg.mozilla.org/mozilla-central/rev/a506ea1db794 https://hg.mozilla.org/mozilla-central/rev/847453f52aab https://hg.mozilla.org/mozilla-central/rev/9a5e8a990f0a

Upstream PR merged

Announced on Firefox 62 for developers: https://developer.mozilla.org/en-US/Firefox/Releases/62#JavaScript New page added: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/import.meta Compat data update: https://github.com/mdn/browser-compat-data/pull/2141 :jonco (or :till), can you let me know if these docs look alright to you? The spec has a neat example (https://github.com/tc39/proposal-import-meta#example) using import.meta.scriptElement, but that's not implemented yet, is it? Do we have a bug for it? Thank you!

(In reply to Florian Scholz [:fscholz] (MDN) from comment #24) > The spec has a neat example > (https://github.com/tc39/proposal-import-meta#example) using > import.meta.scriptElement, but that's not implemented yet, is it? Do we have > a bug for it? That's just an example to justify the proposed introduction of import.meta as a feature. There is no spec for adding this particular property. The html spec defines only the 'url' property: https://html.spec.whatwg.org/multipage/webappapis.html#hostgetimportmetaproperties From the MDN page: "It contains information about the module, like the path of the directory in which the current module is stored and other meta data" As above, it only contains the URL. Apart from that it looks fine.

Awesome, thank you Jon! :)