Skip to content

console implementation according to standard#485

Merged
saghul merged 3 commits intosaghul:masterfrom
lal12:add-missing-console-functions
Apr 12, 2024
Merged

console implementation according to standard#485
saghul merged 3 commits intosaghul:masterfrom
lal12:add-missing-console-functions

Conversation

@lal12
Copy link
Copy Markdown
Contributor

@lal12 lal12 commented Apr 12, 2024

Implementing console according to standard.

Still a bunch of test cases missing, but otherwise complete.

Any thoughts?

@lal12 lal12 force-pushed the add-missing-console-functions branch from 2174e8a to 2304fba Compare April 12, 2024 00:42
@lal12
Copy link
Copy Markdown
Contributor Author

lal12 commented Apr 12, 2024

E.g. would be nice to expose the createConsole function for customization.

Copy link
Copy Markdown
Owner

@saghul saghul left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work! Left some comments.

@lal12 lal12 force-pushed the add-missing-console-functions branch from 2304fba to b42b523 Compare April 12, 2024 09:03
@lal12 lal12 force-pushed the add-missing-console-functions branch from b42b523 to 23add5f Compare April 12, 2024 13:27
@lal12
Copy link
Copy Markdown
Contributor Author

lal12 commented Apr 12, 2024

Added tests from wpt and added some changes to fulfill those and match some nodejs behavior. In some places wpt tests seemed to be wrong, and neither browser nor nodejs acted accordingly (but differently from each other), there I used nodejs behavior.

JS console standards seems to be oddly wild 😆

Diverging from spec are the following things:

  • countReset is resetting instead setting to 0. MDN leaves it open, NodeJS does this too, Chrome+FF follow spec, though FF also prints label: 0
  • added options argument to Logger, to allow for some specific warning handling to better match nodejs behavior

I also copied the code from util package and moved it here to add missing features as defined in console spec. Though alternatively we could extend the alteady existing util patch.

- added wpt test
- pulled code from util package and modified as necessary
- reworked console a bit to work with tests
@lal12 lal12 force-pushed the add-missing-console-functions branch from bfaf6aa to 1311aa4 Compare April 12, 2024 13:36
@saghul
Copy link
Copy Markdown
Owner

saghul commented Apr 12, 2024

I also copied the code from util package and moved it here to add missing features as defined in console spec. Though alternatively we could extend the alteady existing util patch.

Good call, I was thingking about eventually doing that, so thanks for going in that direction.

@saghul
Copy link
Copy Markdown
Owner

saghul commented Apr 12, 2024

This looks great! Are you going to be adding anything else, or should we merge?

@lal12 lal12 marked this pull request as ready for review April 12, 2024 15:07
@lal12
Copy link
Copy Markdown
Contributor Author

lal12 commented Apr 12, 2024

This looks great! Are you going to be adding anything else, or should we merge?

Just pushed the types for createConsole. I have nothing else to add. The only thing I was wondering if there is a better way for registering createConsole (see console.js:327)

@saghul
Copy link
Copy Markdown
Owner

saghul commented Apr 12, 2024

The only thing I was wondering if there is a better way for registering createConsole (see console.js:327)

It looks odd indeed. I'll take a look.

@saghul saghul merged commit d7b8311 into saghul:master Apr 12, 2024
@saghul
Copy link
Copy Markdown
Owner

saghul commented Apr 12, 2024

Excellent work @lal12 !

@lal12 lal12 deleted the add-missing-console-functions branch April 15, 2024 09:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants