Issue 6693 in v8: Implement `import.meta`

kou… via monorail

unread,

Oct 5, 2017, 10:00:21 PM10/5/17

to modul...@chromium.org

Updates:
Cc: hirosh...@chromium.org modul...@chromium.org

Comment #6 on issue 6693 by kou...@chromium.org: Implement `import.meta`
https://bugs.chromium.org/p/v8/issues/detail?id=6693#c6

hiroshige: Would you own the Blink-side impl work?

--
You received this message because:
1. You were specifically CC'd on the issue

You may adjust your notification preferences at:
https://bugs.chromium.org/hosting/settings

Reply to this email to add a comment.

bradley.… via monorail

unread,

Oct 7, 2017, 10:38:13 AM10/7/17

to modul...@chromium.org

Comment #7 on issue 6693 by bradley....@gmail.com: Implement `import.meta`
https://bugs.chromium.org/p/v8/issues/detail?id=6693#c7

The API looks like it accepts arbitrary object while the proposal enforces that it is null prototyped object that can't be reassigned. Is this intentional / was it to allow a Proxy for embedders? I think it is fine, just something to note so embedders don't create something that acts different from a null prototype object.

jan.kr… via monorail

unread,

Oct 7, 2017, 10:42:21 AM10/7/17

to modul...@chromium.org

Comment #8 on issue 6693 by jan.kr...@gmail.com: Implement `import.meta`
https://bugs.chromium.org/p/v8/issues/detail?id=6693#c8

I was wondering the same thing but I think HostFinalizeImportMeta allows to modify the prototype anyhow:

```
> node -p 'Object.getPrototypeOf(Object.setPrototypeOf(Object.create(null), Object))'
[Function: Object]
```

So this shouldn't make a difference.

bradley.… via monorail

unread,

Oct 7, 2017, 10:51:26 AM10/7/17

to modul...@chromium.org

Comment #9 on issue 6693 by bradley....@gmail.com: Implement `import.meta`
https://bugs.chromium.org/p/v8/issues/detail?id=6693#c9

Assigning to the prototype wouldn't change internal fields, Promise.prototype.then would still fail if the Object was not originally a Promise.

jan.kr… via monorail

unread,

Oct 7, 2017, 11:16:55 AM10/7/17

to modul...@chromium.org

Comment #10 on issue 6693 by jan.kr...@gmail.com: Implement `import.meta`
https://bugs.chromium.org/p/v8/issues/detail?id=6693#c10

Sorry, you're right. Forgot about internal fields.

gsat… via monorail

unread,

Oct 7, 2017, 12:32:02 PM10/7/17

to modul...@chromium.org

Comment #11 on issue 6693 by gsat...@chromium.org: Implement `import.meta`
https://bugs.chromium.org/p/v8/issues/detail?id=6693#c11

The API can't always be protected from incorrect embedders -- this is similar to HostImportModuleDynamicallyCallback where the embedder is free to resolve the promise with any javascript object, not just the module namespace object.

I'm happy to add a comment and a DCHECK on the API checking for a JSObject with a null prototype.

bradley.… via monorail

unread,

Oct 7, 2017, 1:01:31 PM10/7/17

to modul...@chromium.org

Comment #12 on issue 6693 by bradley....@gmail.com: Implement `import.meta`
https://bugs.chromium.org/p/v8/issues/detail?id=6693#c12

That resolves my concerns

bugdro… via monorail

unread,

Oct 8, 2017, 10:48:20 PM10/8/17

to modul...@chromium.org

bugdro… via monorail

unread,

Oct 10, 2017, 8:16:50 PM10/10/17

to modul...@chromium.org

Comment #14 on issue 6693 by bugd...@chromium.org: Implement `import.meta`
https://bugs.chromium.org/p/v8/issues/detail?id=6693#c14

The following revision refers to this bug:

bugdro… via monorail

unread,

Oct 10, 2017, 8:16:55 PM10/10/17

to modul...@chromium.org

Comment #15 on issue 6693 by bugd...@chromium.org: Implement `import.meta`
https://bugs.chromium.org/p/v8/issues/detail?id=6693#c15

The following revision refers to this bug:

https://chromium.googlesource.com/v8/v8.git/+/a5565227f2a5eb3821570ab340999817c7d17e77

commit a5565227f2a5eb3821570ab340999817c7d17e77
Author: Sathya Gunasekaran <gsa...@chromium.org>
Date: Wed Oct 11 00:05:13 2017

Revert "[modules] Implement import.meta proposal"

This reverts commit ed6f00fb8e19c541f0a8927222be13eeee89a82c.

Reason for revert: tree is broken

NOTRY=true
NOTREECHECKS=true

Original change's description:

> [modules] Implement import.meta proposal
>
> Rewrites references to import.meta to a new GetImportMetaObject runtime
> call. Embedders can define a callback for creating the meta object using
> v8::Isolate::SetHostGetImportMetaObjectCallback. If no callback has been
> provided, an empty object with null prototype is created.
>
> This adds an example implementation to d8 that sets meta.url.
>
> Bug: v8:6693
> Cq-Include-Trybots: master.tryserver.chromium.linux:linux_chromium_rel_ng
> Change-Id: I6871eec79da45bba81bbbc84b1ffff48534c368d
> Reviewed-on: https://chromium-review.googlesource.com/707902
> Commit-Queue: Sathya Gunasekaran <gsa...@chromium.org>
> Reviewed-by: Adam Klein <ad...@chromium.org>
> Reviewed-by: Sathya Gunasekaran <gsa...@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#48433}

TBR=ad...@chromium.org,gsa...@chromium.org,jan....@groupon.com

Change-Id: I908a508d5db84cc8ae60d4fd4a0446bb570c1492
No-Presubmit: true
No-Tree-Checks: true
No-Try: true

Bug: v8:6693
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_chromium_rel_ng

bugdro… via monorail

unread,

Oct 11, 2017, 11:37:33 AM10/11/17

to modul...@chromium.org

Comment #16 on issue 6693 by bugd...@chromium.org: Implement `import.meta`
https://bugs.chromium.org/p/v8/issues/detail?id=6693#c16

The following revision refers to this bug:

https://chromium.googlesource.com/v8/v8.git/+/1c5529993dfd0afd3545708a41896456c0d25d70

commit 1c5529993dfd0afd3545708a41896456c0d25d70
Author: Jan Krems <jan....@groupon.com>
Date: Wed Oct 11 15:37:10 2017

Reland "[modules] Implement import.meta proposal"

This is a reland of ed6f00fb8e19c541f0a8927222be13eeee89a82c

Original change's description:
> [modules] Implement import.meta proposal
>
> Rewrites references to import.meta to a new GetImportMetaObject runtime
> call. Embedders can define a callback for creating the meta object using
> v8::Isolate::SetHostGetImportMetaObjectCallback. If no callback has been
> provided, an empty object with null prototype is created.
>
> This adds an example implementation to d8 that sets meta.url.
>
> Bug: v8:6693
> Cq-Include-Trybots: master.tryserver.chromium.linux:linux_chromium_rel_ng
> Change-Id: I6871eec79da45bba81bbbc84b1ffff48534c368d
> Reviewed-on: https://chromium-review.googlesource.com/707902
> Commit-Queue: Sathya Gunasekaran <gsa...@chromium.org>
> Reviewed-by: Adam Klein <ad...@chromium.org>
> Reviewed-by: Sathya Gunasekaran <gsa...@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#48433}

TBR=ad...@chromium.org

Bug: v8:6693
Change-Id: Ie2d746ad996a56ed6ff50b832f320fe44e02f231
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_chromium_rel_ng
Reviewed-on: https://chromium-review.googlesource.com/712834

Reviewed-by: Sathya Gunasekaran <gsa...@chromium.org>
Commit-Queue: Sathya Gunasekaran <gsa...@chromium.org>

gsat… via monorail

unread,

Oct 11, 2017, 12:17:21 PM10/11/17

to modul...@chromium.org

Comment #17 on issue 6693 by gsat...@chromium.org: Implement `import.meta`
https://bugs.chromium.org/p/v8/issues/detail?id=6693#c17

Jan, are you interested in working on optimizing import.meta? I've updated the design doc.

jan.kr… via monorail

unread,

Oct 11, 2017, 3:52:31 PM10/11/17

to modul...@chromium.org

Comment #19 on issue 6693 by jan.kr...@gmail.com: Implement `import.meta`
https://bugs.chromium.org/p/v8/issues/detail?id=6693#c19

The short answer is: I'm definitely interested!

The disclaimer would be that I had a basic reading comprehension and/or prior experience working on parsers but the same isn't true for the runtime bits. So I'm not sure how efficient I'd be at working on it. On the other hand the description in the design doc looks like it has enough hints to figure it out from there and it would be a welcome opportunity to better learn how I/TF works.

gsat… via monorail

unread,

Oct 11, 2017, 5:19:18 PM10/11/17

to modul...@chromium.org

Comment #20 on issue 6693 by gsat...@chromium.org: Implement `import.meta`
https://bugs.chromium.org/p/v8/issues/detail?id=6693#c20

That's totally fine. I'm happy to help you through it, let me know if you get stuck.

jan.kr… via monorail

unread,

Oct 11, 2017, 9:10:28 PM10/11/17

to modul...@chromium.org

Comment #21 on issue 6693 by jan.kr...@gmail.com: Implement `import.meta`
https://bugs.chromium.org/p/v8/issues/detail?id=6693#c21

I just discovered that lazy-parsed functions (?) don't work properly yet. The following fails with a SyntaxError:

```
const loadImportMetaArrow = () => import.meta;
function loadImportMetaFn() { return import.meta; };
assertSame(loadImportMetaArrow(), import.meta);
assertSame(loadImportMetaFn(), import.meta);
```

Should I fix that in a separate CL or is it okay to do it in one CL with the optimization changes?

gsat… via monorail

unread,

Oct 11, 2017, 9:39:25 PM10/11/17

to modul...@chromium.org

Comment #22 on issue 6693 by gsat...@chromium.org: Implement `import.meta`
https://bugs.chromium.org/p/v8/issues/detail?id=6693#c22

Thanks! Filed https://bugs.chromium.org/p/v8/issues/detail?id=6919 to track this. This can be fixed independently.

jan.kr… via monorail

unread,

Oct 12, 2017, 11:06:36 AM10/12/17

to modul...@chromium.org

Comment #23 on issue 6693 by jan.kr...@gmail.com: Implement `import.meta`
https://bugs.chromium.org/p/v8/issues/detail?id=6693#c23

Uploaded a WIP version to https://chromium-review.googlesource.com/c/v8/v8/+/716536. From what I can tell the bytecode & its interpretation are doing their job. It's only calling the Runtime_* function for the first access to import.meta (didn't update the Runtime function implementation yet to DCHECK(IsHole) etc.).

The thing I'm struggling with is to figure out how to trigger the BytecodeGraphBuilder code. That's where I ran into the lazy parsed functions issue (trying to make import.meta appear in a hot function). Are the unit tests for the graph builder the best bet here?

gsat… via monorail

unread,

Oct 12, 2017, 4:59:50 PM10/12/17

to modul...@chromium.org

Comment #24 on issue 6693 by gsat...@chromium.org: Implement `import.meta`
https://bugs.chromium.org/p/v8/issues/detail?id=6693#c24

Thanks! Commented on the CL.

jan.kr… via monorail

unread,

Oct 12, 2017, 8:18:13 PM10/12/17

to modul...@chromium.org

Comment #25 on issue 6693 by jan.kr...@gmail.com: Implement `import.meta`
https://bugs.chromium.org/p/v8/issues/detail?id=6693#c25

I was pinged fuzzer crash (from what I understand): https://bugs.chromium.org/p/chromium/issues/detail?id=774120. Can't access the report. Is there anything I can/should be doing?

gsat… via monorail

unread,

Oct 12, 2017, 8:22:54 PM10/12/17

to modul...@chromium.org

Comment #26 on issue 6693 by gsat...@chromium.org: Implement `import.meta`
https://bugs.chromium.org/p/v8/issues/detail?id=6693#c26

Thanks, I'll take a look.

bugdro… via monorail

unread,

Oct 20, 2017, 5:37:55 PM10/20/17

to modul...@chromium.org

Comment #27 on issue 6693 by bugd...@chromium.org: Implement `import.meta`
https://bugs.chromium.org/p/v8/issues/detail?id=6693#c27

The following revision refers to this bug:

https://chromium.googlesource.com/v8/v8.git/+/86832486259808c734953bb18803d8981db4d01d

commit 86832486259808c734953bb18803d8981db4d01d
Author: Jan Krems <jan....@groupon.com>
Date: Fri Oct 20 20:31:49 2017

[modules] Optimize import.meta in the interpreter

Use an intrinsic for GetImportMetaObject and generate bytecode for the
case where import.meta has been initialized already. This way the
runtime method will only be called once per module.

Bug: v8:6693
Change-Id: If661e88e6accfb1c5795e37a80582d04f6dd87dd
Reviewed-on: https://chromium-review.googlesource.com/716536
Reviewed-by: Sathya Gunasekaran <gsa...@chromium.org>
Reviewed-by: Ross McIlroy <rmci...@chromium.org>
Reviewed-by: Marja Hölttä <ma...@chromium.org>
Commit-Queue: Sathya Gunasekaran <gsa...@chromium.org>
Cr-Commit-Position: refs/heads/master@{#48801}
[modify] https://crrev.com/86832486259808c734953bb18803d8981db4d01d/AUTHORS
[modify] https://crrev.com/86832486259808c734953bb18803d8981db4d01d/src/code-stub-assembler.cc
[modify] https://crrev.com/86832486259808c734953bb18803d8981db4d01d/src/code-stub-assembler.h
[modify] https://crrev.com/86832486259808c734953bb18803d8981db4d01d/src/interpreter/interpreter-intrinsics-generator.cc
[modify] https://crrev.com/86832486259808c734953bb18803d8981db4d01d/src/interpreter/interpreter-intrinsics.h
[modify] https://crrev.com/86832486259808c734953bb18803d8981db4d01d/src/parsing/parser.cc
[modify] https://crrev.com/86832486259808c734953bb18803d8981db4d01d/test/mjsunit/harmony/modules-import-meta.js

gsat… via monorail

unread,

Nov 6, 2017, 5:03:20 PM11/6/17

to modul...@chromium.org

Updates:
Status: Fixed

Comment #28 on issue 6693 by gsat...@chromium.org: Implement `import.meta`
https://bugs.chromium.org/p/v8/issues/detail?id=6693#c28

Filed https://bugs.chromium.org/p/v8/issues/detail?id=7044 to track optimization of import.meta.

Closing this as import.meta is implemented and shipping in Chrome 64.

bugdro… via monorail

unread,

Jan 26, 2018, 4:41:03 PM1/26/18

to modul...@chromium.org

Comment #29 on issue 6693 by bugd...@chromium.org: Implement `import.meta`
https://bugs.chromium.org/p/v8/issues/detail?id=6693#c29

The following revision refers to this bug:

https://chromium.googlesource.com/v8/v8.git/+/6ff29275239ae117aa42ad3daa6a17a46d0e20c4

commit 6ff29275239ae117aa42ad3daa6a17a46d0e20c4
Author: Sathya Gunasekaran <gsa...@chromium.org>
Date: Fri Jan 26 21:40:34 2018

[ESNext] Ship import() and import.meta

import() is shipping by default in Chrome 63 and import.meta is
shipping in Chrome 64 already.

The flag still exists so that embedders can turn off the feature
if required.

Bug: v8:5785, v8:6693
Change-Id: Ie6c8f90927958022900f677ea5a771527111aece
Reviewed-on: https://chromium-review.googlesource.com/889658
Commit-Queue: Sathya Gunasekaran <gsa...@chromium.org>
Reviewed-by: Adam Klein <ad...@chromium.org>
Cr-Commit-Position: refs/heads/master@{#50913}
[modify] https://crrev.com/6ff29275239ae117aa42ad3daa6a17a46d0e20c4/src/flag-definitions.h

bugdroid via monorail

unread,

Nov 3, 2020, 1:06:35 PM11/3/20

to modul...@chromium.org

Comment #30 on issue 6693 by bugdroid: Implement `import.meta`
https://bugs.chromium.org/p/v8/issues/detail?id=6693#c30

The following revision refers to this bug:
  

https://chromium.googlesource.com/v8/v8.git/+/a5e33a57d794d42c62234f1c4d0a4b51793e98af

commit a5e33a57d794d42c62234f1c4d0a4b51793e98af
Author: Shu-yu Guo <s...@chromium.org>
Date: Tue Nov 03 18:05:52 2020

[flags] Remove --harmony-import-meta

It's shipped since M64.

Bug: v8:6693
Change-Id: Ifc547c0036dd7e5f7bab9bcb169abee0157f73b3
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2509943
Commit-Queue: Shu-yu Guo <s...@chromium.org>
Reviewed-by: Marja Hölttä <ma...@chromium.org>
Cr-Commit-Position: refs/heads/master@{#70954}

[modify] https://crrev.com/a5e33a57d794d42c62234f1c4d0a4b51793e98af/src/flags/flag-definitions.h
[modify] https://crrev.com/a5e33a57d794d42c62234f1c4d0a4b51793e98af/src/init/bootstrapper.cc
[modify] https://crrev.com/a5e33a57d794d42c62234f1c4d0a4b51793e98af/src/parsing/parse-info.cc
[modify] https://crrev.com/a5e33a57d794d42c62234f1c4d0a4b51793e98af/src/parsing/parse-info.h
[modify] https://crrev.com/a5e33a57d794d42c62234f1c4d0a4b51793e98af/src/parsing/parser-base.h
[modify] https://crrev.com/a5e33a57d794d42c62234f1c4d0a4b51793e98af/src/parsing/parser.cc
[modify] https://crrev.com/a5e33a57d794d42c62234f1c4d0a4b51793e98af/test/cctest/test-api.cc
[modify] https://crrev.com/a5e33a57d794d42c62234f1c4d0a4b51793e98af/test/cctest/test-parsing.cc

bugdroid via monorail

unread,

Nov 4, 2020, 11:02:36 AM11/4/20

to modul...@chromium.org

Comment #31 on issue 6693 by bugdroid: Implement `import.meta`
https://bugs.chromium.org/p/v8/issues/detail?id=6693#c31

The following revision refers to this bug:
  

https://chromium.googlesource.com/chromium/src.git/+/c475d17d31edc18f2f6e4ee0299a064272a05451

commit c475d17d31edc18f2f6e4ee0299a064272a05451
Author: Sigurd Schneider <sig...@chromium.org>
Date: Wed Nov 04 15:55:28 2020

Remove mentions of old V8 flags from Chromium

Flags removed:
  --harmony-import-meta https://crrev.com/c/2509943
  --harmony-dynamic-import https://crrev.com/c/2509942

Bug: v8:6693, v8:5785
Change-Id: I0628db7d48f4991c185fd6f8e9f49d5f7eb91094
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2517685
Reviewed-by: Kentaro Hara <har...@chromium.org>
Commit-Queue: Sigurd Schneider <sig...@chromium.org>
Cr-Commit-Position: refs/heads/master@{#823995}

[modify] https://crrev.com/c475d17d31edc18f2f6e4ee0299a064272a05451/content/renderer/render_process_impl.cc

Reply all

Reply to author

Forward