New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: Implement all the web console methods #522

Merged

JakeChampion merged 5 commits into main from jake/console

May 11, 2023

Contributor

JakeChampion commented May 10, 2023 •

edited

Corrects the function lengths for all the methods to be 0 instead of 1 as all the functions parameters are optional.

Adds a Symbol.toStringTag property with a value of "console".

Fixes the prototype chain for globalThis.console to match the specification.

Bringing in implementations for:

console.assert -- This returns true is the first parameter is truthy. If first param is falsey then this writes to Error
console.clear - This is a no-op :D
console.count - Increments a counter for the provided label, if no label provided, then the label "default" is used.
console.countReset - Resets a counter for the provided label, if no label provided, then the label "default" is used.
console.dir - Exactly the same as console.log
console.dirxml - Exactly the same as console.log
console.group - Exactly the same as console.log
console.groupCollapsed - Exactly the same as console.log
console.groupEnd - This is a no-op :D
console.table - Exactly the same as console.log
console.time - Adds the provided label to the timer map for use with timeLog or timeEnd, if no label provided, then the label "default" is used.
console.timeLog - Logs the current value of a timer that was previously started by calling console.time().
console.timeEnd - The console.timeEnd() stops a timer that was previously started by calling console.time().
console.trace - Logs a stack trace.

JakeChampion requested a review from elliottt

May 10, 2023 10:29

JakeChampion force-pushed the jake/console branch 7 times, most recently from 8544253 to 8ef289a Compare

May 10, 2023 10:36

JakeChampion requested a review from guybedford

May 10, 2023 16:32

guybedford approved these changes

View reviewed changes

Contributor

guybedford left a comment

Great stuff

runtime/js-compute-runtime/builtins/shared/console.cpp Outdated

+                std::string concat = label;
+                concat += ": ";
+                concat += std::to_string(duration);
+                concat += "ns";

Contributor

guybedford May 10, 2023

Typically this seems to work in seconds (node.js) or milliseconds (chrome / deno). With nanoseconds, this is 9 orders of magnitude smaller than milliseconds. Perhaps we should do some rounding or at least add a decimal place?

Contributor Author

JakeChampion May 11, 2023

node.js does dynamic formatting, it will use ms first and then if/when the period increases it will move to seconds/minutes/etc

Firefox always uses ms
Safari always uses fractional ms

I've updated our implementation to always use fractional ms 👍

runtime/js-compute-runtime/builtins/shared/console.cpp

+                  JS_FN("dir", dir, 0, JSPROP_ENUMERATE),
+                  JS_FN("dirxml", (console_out<Console::LogType::Log>), 0, JSPROP_ENUMERATE),
+                  JS_FN("error", (console_out<Console::LogType::Error>), 0, JSPROP_ENUMERATE),
+                  JS_FN("group", (console_out<Console::LogType::Log>), 0, JSPROP_ENUMERATE),

Contributor

guybedford May 10, 2023

Node.js and Deno have an indentation counter for grouping, although doesn't seem to be that valuable a feature to skip for now to me.

runtime/js-compute-runtime/builtins/shared/console.cpp

+                  return false;
+                }
+                size_t length;
+                auto stack_string = encode(cx, str, &length);

Contributor

guybedford May 10, 2023

Great!

runtime/js-compute-runtime/builtins/shared/console.cpp Outdated

Comment on lines 469 to 483

+                    // strip quotes for direct string logs
+                    if (source[0] == '"' && source[source.length() - 1] == '"') {
+                      source = source.substr(1, source.length() - 2);
+                    }
+                    if (message.length()) {
+                      message += " ";
+                      message += source;
+                    } else {
+                      message += source;
+                    }

Contributor

guybedford May 10, 2023

This pattern is repeated quite a lot, perhaps let's just refactor it into a separate function?

elliottt approved these changes

View reviewed changes

Contributor

elliottt left a comment

Looks good to me! Just a few suggestions for map query/update patterns.

runtime/js-compute-runtime/builtins/shared/console.cpp Outdated Show resolved Hide resolved

runtime/js-compute-runtime/builtins/shared/console.cpp Outdated Show resolved Hide resolved

runtime/js-compute-runtime/builtins/shared/console.cpp Outdated Show resolved Hide resolved

runtime/js-compute-runtime/builtins/shared/console.cpp Outdated Show resolved Hide resolved

runtime/js-compute-runtime/builtins/shared/console.cpp Outdated Show resolved Hide resolved

JakeChampion force-pushed the main branch 7 times, most recently from 9d26f5a to 5198884 Compare

May 11, 2023 10:14

JakeChampion and others added 4 commits

May 11, 2023 15:34


          feat: Implement all the web console methods

cadd2b4

Corrects the function lengths for all the methods to be `0` instead of `1` as all the functions parameters are optional.

Adds a Symbol.toStringTag property with a value of `"console"`.

Fixes the prototype chain for `globalThis.console` to match the specification.

Bringing in implementations for:
- console.assert
- console.clear
- console.count
- console.countReset
- console.dir
- console.dirxml
- console.group
- console.groupCollapsed
- console.groupEnd
- console.table
- console.time
- console.timeLog
- console.timeEnd
- console.trace


          Apply suggestions from code review

f255048

Co-authored-by: Trevor Elliott <telliott@fastly.com>


          chore: use fractional milliseconds for the console.time methods

3d4b76c


          chore: simplify assert implemention by calling args.rval().setUndefin…

43ef438

…ed() once

JakeChampion force-pushed the jake/console branch from 57057f9 to 43ef438 Compare

May 11, 2023 14:34


          place the formatter into anonymous namespace

dc20760

JakeChampion merged commit a12a1d3 into main

77 checks passed

JakeChampion deleted the jake/console branch

May 11, 2023 15:00

JakeChampion mentioned this pull request

chore(main): release 1.13.0 #527

Merged

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment