Add API to get all the dependencies of a given JSScript
Created attachment 373997 [details] Patch
Created attachment 373998 [details] Patch
rdar://problem/51768389
Attachment 373998 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/API/tests/testapi.mm:2646: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/API/tests/testapi.mm:2672: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/API/tests/testapi.mm:2692: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/API/tests/testapi.mm:2712: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/API/tests/testapi.mm:2732: Place brace on its own line for function definitions. [whitespace/braces] [4] Total errors found: 5 in 19 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 373998 [details] Patch Attachment 373998 [details] did not pass jsc-ews (mac): Output: https://webkit-queues.webkit.org/results/12723132 New failing tests: mozilla-tests.yaml/js1_5/Array/regress-101964.js.mozilla-dfg-eager-no-cjit-validate-phases mozilla-tests.yaml/js1_5/Array/regress-101964.js.mozilla-ftl-eager-no-cjit-validate-phases apiTests
Created attachment 374020 [details] Patch
Attachment 374020 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/API/tests/testapi.mm:2646: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/API/tests/testapi.mm:2672: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/API/tests/testapi.mm:2692: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/API/tests/testapi.mm:2712: Place brace on its own line for function definitions. [whitespace/braces] [4] ERROR: Source/JavaScriptCore/API/tests/testapi.mm:2732: Place brace on its own line for function definitions. [whitespace/braces] [4] Total errors found: 5 in 20 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 374020 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=374020&action=review > Source/JavaScriptCore/ChangeLog:10 > + are at the satify phase but for API simplicity we only provide satify => satisfy > Source/JavaScriptCore/ChangeLog:15 > + This patch also fixes an issue where we would allow import specifiers > + that didn't start "./" or "/". why is that bad? "foo" just just be "./foo", right? > Source/JavaScriptCore/API/JSAPIGlobalObject.mm:72 > +static Expected<URL, String> computeValidImportSpecifier(const URL& base, const String& specifier) "computeValidImportSpecifier" => "computeURL"? > Source/JavaScriptCore/API/JSAPIGlobalObject.mm:79 > + if (!specifier.startsWith('/') && !specifier.startsWith("./") && !specifier.startsWith("../")) > + return makeUnexpected(makeString("Module specifier: "_s, specifier, " does not start with \"/\", \"./\", or \"../\"."_s)); I don't see why this is necessary. See comment in changelog > Source/JavaScriptCore/API/JSContext.mm:163 > + JSC::JSArray* result = exec->lexicalGlobalObject()->moduleLoader()->dependencyKeysIfEvaluated(exec, JSC::jsString(&vm, [[script sourceURL] absoluteString])); > + if (!result) { Don't you want an exception scope for this?
Comment on attachment 374020 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=374020&action=review >> Source/JavaScriptCore/ChangeLog:15 >> + that didn't start "./" or "/". > > why is that bad? "foo" just just be "./foo", right? Please describe why we're doing this? E.g, to be compatible with web. And why the web does it > Source/JavaScriptCore/ChangeLog:18 > + Lastly, this patch makes it so that we copy all scripts in the API/tests/testapiScripts directory so > + they don't have to be individually added to the xcode project. 👍🏼
Comment on attachment 374020 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=374020&action=review >> Source/JavaScriptCore/API/JSContext.mm:163 >> + if (!result) { > > Don't you want an exception scope for this? Uhh, I don't think there's any non-internal exceptions that can be thrown right now but maybe it's good practice.
Created attachment 374023 [details] Patch for landing
Committed r247403: <https://trac.webkit.org/changeset/247403>