Bug 214379

Summary: Support export namespace `export * as ns`
Product: WebKit Reporter: Huáng Jùnliàng <jlhwung>
Component: JavaScriptCoreAssignee: Yusuke Suzuki <ysuzuki>
Status: RESOLVED FIXED    
Severity: Normal CC: ews-watchlist, fpizlo, keith_miller, mark.lam, msaboff, ross.kirsling, saam, tzagallo, webkit-bug-importer, ysuzuki
Priority: P2 Keywords: InRadar
Version: Safari Technology Preview   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
ross.kirsling: review+
Patch
ews-feeder: commit-queue-
Patch none

Description Huáng Jùnliàng 2020-07-15 14:33:42 PDT 
`export * as ns` is included in ES2020.

Expect: it works on Safari

Actual: Safari TP 109 throws warning that missing "from" after `*`.

Spec PR: https://github.com/tc39/ecma262/pull/1174
Comment 1 Radar WebKit Bug Importer 2020-07-16 18:04:13 PDT 
<rdar://problem/65699863>
Comment 2 Yusuke Suzuki 2020-09-16 01:33:31 PDT 
Created attachment 408905 [details]
Patch

WIP
Comment 3 Yusuke Suzuki 2020-09-16 22:26:35 PDT 
Created attachment 408990 [details]
Patch
Comment 4 Ross Kirsling 2020-09-16 23:27:55 PDT 
Comment on attachment 408990 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=408990&action=review

r=me. Thanks for the extra explanation over Slack.

> Source/JavaScriptCore/parser/NodesAnalyzeModule.cpp:89
> +            // export { * as v } from "mod"

This form can't use braces though, right?

> Source/JavaScriptCore/runtime/AbstractModuleRecord.cpp:779
> +    // Materialize *namespace* slot with module namespace object.
> +    // If module environment is not yet materialized, we will materialize it when materializing module environment.

The "it" here is a bit confusing upon first read. Maybe something like "...unless the module environment is not yet materialized, in which case we'll do it in setModuleEnvironment"?

> Source/JavaScriptCore/runtime/AbstractModuleRecord.cpp:782
> +        bool putResult = false;
> +        symbolTablePutTouchWatchpointSet(m_moduleEnvironment.get(), globalObject, vm.propertyNames->starNamespacePrivateName, moduleNamespaceObject, /* shouldThrowReadOnlyError */ false, /* ignoreReadOnlyErrors */ true, putResult);

nit: I know this is copied code, but it seems silly that one boolean is described using an identifier and the other two are described with block comments.
Comment 5 Yusuke Suzuki 2020-09-17 01:05:17 PDT 
Created attachment 409007 [details]
Patch
Comment 6 Yusuke Suzuki 2020-09-17 01:09:21 PDT 
Created attachment 409008 [details]
Patch
Comment 7 Yusuke Suzuki 2020-09-17 03:27:51 PDT 
Comment on attachment 408990 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=408990&action=review

>> Source/JavaScriptCore/parser/NodesAnalyzeModule.cpp:89
>> +            // export { * as v } from "mod"
> 
> This form can't use braces though, right?

Right, fixed.

>> Source/JavaScriptCore/runtime/AbstractModuleRecord.cpp:779
>> +    // If module environment is not yet materialized, we will materialize it when materializing module environment.
> 
> The "it" here is a bit confusing upon first read. Maybe something like "...unless the module environment is not yet materialized, in which case we'll do it in setModuleEnvironment"?

Sounds good!

>> Source/JavaScriptCore/runtime/AbstractModuleRecord.cpp:782
>> +        symbolTablePutTouchWatchpointSet(m_moduleEnvironment.get(), globalObject, vm.propertyNames->starNamespacePrivateName, moduleNamespaceObject, /* shouldThrowReadOnlyError */ false, /* ignoreReadOnlyErrors */ true, putResult);
> 
> nit: I know this is copied code, but it seems silly that one boolean is described using an identifier and the other two are described with block comments.

Fixed.
Comment 8 Yusuke Suzuki 2020-09-17 03:28:18 PDT 
iOS-wk2 failures are unrelated.
https://ews-build.webkit.org/#/builders/24/builds/25994
Comment 9 Yusuke Suzuki 2020-09-17 03:30:24 PDT 
Committed r267186: <https://trac.webkit.org/changeset/267186>