Summary: | [JSC] Prototype dynamic-import | ||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Yusuke Suzuki <ysuzuki> | ||||||||||||||||||||||||
Component: | JavaScriptCore | Assignee: | Yusuke Suzuki <ysuzuki> | ||||||||||||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||||||||||||
Severity: | Normal | CC: | buildbot, chi187, commit-queue, darin, keith_miller, mark.lam, msaboff, rniwa, saam, sam, sgospodarets | ||||||||||||||||||||||||
Priority: | P2 | ||||||||||||||||||||||||||
Version: | WebKit Nightly Build | ||||||||||||||||||||||||||
Hardware: | Unspecified | ||||||||||||||||||||||||||
OS: | Unspecified | ||||||||||||||||||||||||||
Bug Depends on: | 164425, 164860 | ||||||||||||||||||||||||||
Bug Blocks: | 166488, 166926 | ||||||||||||||||||||||||||
Attachments: |
|
Description
Yusuke Suzuki
2016-12-10 03:33:07 PST
Created attachment 297763 [details]
Patch
prototype
Comment on attachment 297763 [details] Patch Attachment 297763 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/2789323 New failing tests: sputnik/Conformance/07_Lexical_Conventions/7.5_Tokens/7.5.3_Future_Reserved_Words/S7.5.3_A1.16.html Created attachment 297764 [details]
Archive of layout-test-results from ews102 for mac-elcapitan
The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews102 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Comment on attachment 297763 [details] Patch Attachment 297763 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/2789336 New failing tests: sputnik/Conformance/07_Lexical_Conventions/7.5_Tokens/7.5.3_Future_Reserved_Words/S7.5.3_A1.16.html Created attachment 297765 [details]
Archive of layout-test-results from ews106 for mac-elcapitan-wk2
The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews106 Port: mac-elcapitan-wk2 Platform: Mac OS X 10.11.6
Comment on attachment 297763 [details] Patch Attachment 297763 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/2789332 New failing tests: sputnik/Conformance/07_Lexical_Conventions/7.5_Tokens/7.5.3_Future_Reserved_Words/S7.5.3_A1.16.html Created attachment 297766 [details]
Archive of layout-test-results from ews112 for mac-elcapitan
The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews112 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Comment on attachment 297763 [details] Patch Attachment 297763 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/2789333 New failing tests: sputnik/Conformance/07_Lexical_Conventions/7.5_Tokens/7.5.3_Future_Reserved_Words/S7.5.3_A1.16.html Created attachment 297767 [details]
Archive of layout-test-results from ews123 for ios-simulator-wk2
The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews123 Port: ios-simulator-wk2 Platform: Mac OS X 10.11.6
Created attachment 297768 [details]
Patch
prototype
Created attachment 297771 [details]
Patch
WIP
Created attachment 297780 [details]
Patch
Created attachment 297782 [details]
Patch
Created attachment 297783 [details]
Patch
Comment on attachment 297783 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=297783&action=review r=me > Source/JavaScriptCore/parser/Parser.cpp:4390 > + handleProductionOrFail2(OPENPAREN, "(", "start", "'import' call"); I think a better error message here would be something like: "import call expects exactly one argument" > Source/JavaScriptCore/parser/Parser.cpp:4393 > + handleProductionOrFail2(CLOSEPAREN, ")", "end", "'import' call"); ditto > Source/JavaScriptCore/runtime/JSGlobalObjectFunctions.cpp:940 > + RETURN_IF_EXCEPTION(catchScope, encodedJSValue()); Nit: you can use "{ }" instead of "encodedJSValue()" > Source/JavaScriptCore/runtime/JSGlobalObjectFunctions.cpp:948 > + auto* specifier = exec->argument(0).toString(exec); Nit: Since this is just called from our own bytecode, I would RELEASE_ASSERT the argument count and use uncheckedArgument. > JSTests/stress/import-syntax.js:25 > +testSyntaxError(`import("Hello", "Hello2";`, `SyntaxError: Unexpected token ','. Expected ')' to end an 'import' call.`); Some more syntax error tests to add: import(a, b) import(a, b, c) import(...foo) I would also suggest adding tests that assert a synax error *was not* thrown. That way we can also test what we allow. Created attachment 298382 [details]
Patch for landing
Patch for landing
Committed r210522: <http://trac.webkit.org/changeset/210522> The dynamic import() doesn't work in the DevTools console. Even for the correct absolute URLs it throws an error: "Cannot resolve the specifier". Is it the intentional behavior, or a bug (just want to confirm before opening a new issue). (In reply to comment #18) > The dynamic import() doesn't work in the DevTools console. > Even for the correct absolute URLs it throws an error: "Cannot resolve the > specifier". > Is it the intentional behavior, or a bug (just want to confirm before > opening a new issue). It's intentional currently. And once this[1] patch is landed, it will be fixed. :) [1]: https://bugs.webkit.org/show_bug.cgi?id=167457 |