Skip to content
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.

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

doc: explain differences in console.assert between node and browsers #6169

Closed
wants to merge 2 commits into from

Conversation

jasnell
Copy link
Member

@jasnell jasnell commented Apr 12, 2016

Checklist
  • documentation is changed or added
  • the commit message follows commit guidelines
Affected core subsystem(s)

doc, console

Description of change

Provide an example for implementing browser like behavior for console.assert.

This "fixes" #5340 by providing an alternative to changing Node.js' implemented behavior. Instead, we document the differences and show how to work around them if browser like semantics are desired.

Fixes: #5340

@jasnell jasnell added doc Issues and PRs related to the documentations. console Issues and PRs related to the console subsystem. labels Apr 12, 2016
@jasnell
Copy link
Member Author

jasnell commented Apr 13, 2016

@nodejs/documentation

```js
'use strict';

const myConsole = Object.setPrototypeOf({
Copy link
Member

Choose a reason for hiding this comment

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

Why do we need a setPrototypeOf call here?

Copy link
Member Author

Choose a reason for hiding this comment

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

actually, the example is incomplete... just noticed... it should be:

const myConsole = Object.setPrototypeOf({
   assert(assertion, message, ...args) {
     try {
      console.assert(assertion, message, ...args);
     } catch (err) {
       console.error(err.stack);
    }
   }
 }, console);

Essentially, this creates a simple extension of the existing console object without monkey-patching it.

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed the example in updated commit

Copy link
Member

Choose a reason for hiding this comment

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

Not sure we should be encouraging such liberal use of setPrototypeOf. A lot of people have strong feelings against it and it sort of lumped in the language by mistake (it's a funny story really).

If we are using it though, why not just:

const myConsole = { __proto__: console,
                    assert: ... }

Copy link
Member Author

Choose a reason for hiding this comment

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

Not sure encouraging use of __proto__ is any better.

Provide an example for implementing browser like behavior for console.assert.

This "fixes" nodejs#5340 by providing an
alternative to changing Node.js' implemented behavior. Instead, we
document the differences and show how to work around them if
browser like semantics are desired.

Fixes: nodejs#5340
@jasnell
Copy link
Member Author

jasnell commented Apr 18, 2016

ping @nodejs/documentation

@@ -109,6 +109,45 @@ console.assert(false, 'Whoops %s', 'didn\'t work');
// AssertionError: Whoops didn't work
```

It is important to note that the `console.assert()` method in Node.js is
implemented differently than the `console.assert()` method available in
Copy link
Contributor

Choose a reason for hiding this comment

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

@eljefedelrodeodeljefe
Copy link
Contributor

LGTM with a nit.

@techjeffharris
Copy link
Contributor

A couple links suggested, but LGTM

@jasnell
Copy link
Member Author

jasnell commented Apr 18, 2016

@eljefedelrodeodeljefe @techjeffharris ... updated, PTAL!

@techjeffharris
Copy link
Contributor

Though this PR is only targeting a clarification of console.assert, as log as there's a link to the browser console.assert maybe we should have a link to the general browser console API in the following section: (sorry, not in a place to easily see which line)

While the API for the Console class is designed fundamentally around the browser console object, the Console in Node.js is not intended to duplicate the browser's functionality exactly.

Might be more appropriate in another PR...?

@techjeffharris
Copy link
Contributor

The most recent changes LGTM, regardless.

@eljefedelrodeodeljefe
Copy link
Contributor

LGTM. Thx @jasnell. @techjeffharris let's collect those in the wake of nodejs/docs#100 (?)

jasnell added a commit that referenced this pull request Apr 18, 2016
Provide an example for implementing browser like behavior for console.assert.

This "fixes" #5340 by providing an
alternative to changing Node.js' implemented behavior. Instead, we
document the differences and show how to work around them if
browser like semantics are desired.

Fixes: #5340
PR-URL: #6169
Reviewed-By: Robert Jefe Lindstädt <robert.lindstaedt@gmail.com>
Reviewed-By: Jeff Harris <@techjeffharris>
@jasnell
Copy link
Member Author

jasnell commented Apr 18, 2016

Landed in 40a5761

@jasnell jasnell closed this Apr 18, 2016
MylesBorins pushed a commit that referenced this pull request Apr 19, 2016
Provide an example for implementing browser like behavior for console.assert.

This "fixes" #5340 by providing an
alternative to changing Node.js' implemented behavior. Instead, we
document the differences and show how to work around them if
browser like semantics are desired.

Fixes: #5340
PR-URL: #6169
Reviewed-By: Robert Jefe Lindstädt <robert.lindstaedt@gmail.com>
Reviewed-By: Jeff Harris <@techjeffharris>
MylesBorins pushed a commit that referenced this pull request Apr 20, 2016
Provide an example for implementing browser like behavior for console.assert.

This "fixes" #5340 by providing an
alternative to changing Node.js' implemented behavior. Instead, we
document the differences and show how to work around them if
browser like semantics are desired.

Fixes: #5340
PR-URL: #6169
Reviewed-By: Robert Jefe Lindstädt <robert.lindstaedt@gmail.com>
Reviewed-By: Jeff Harris <@techjeffharris>
MylesBorins pushed a commit that referenced this pull request Apr 20, 2016
Provide an example for implementing browser like behavior for console.assert.

This "fixes" #5340 by providing an
alternative to changing Node.js' implemented behavior. Instead, we
document the differences and show how to work around them if
browser like semantics are desired.

Fixes: #5340
PR-URL: #6169
Reviewed-By: Robert Jefe Lindstädt <robert.lindstaedt@gmail.com>
Reviewed-By: Jeff Harris <@techjeffharris>
@MylesBorins MylesBorins mentioned this pull request Apr 20, 2016
@qfox
Copy link

qfox commented Apr 20, 2016

This definitely NOT fixes #5340. Sad news everyone.
Can you please explain why you prefer non-standard behaviour?

@jasnell
Copy link
Member Author

jasnell commented Apr 20, 2016

The way browsers implement console is not standard (see: https://developer.mozilla.org/en-US/docs/Web/API/Console). This has been discussed before and it has been decided that it is not a priority for Node.js to align with the browsers on the non-standardized behavior of console.

@qfox
Copy link

qfox commented Apr 20, 2016

Thank you.

MylesBorins pushed a commit that referenced this pull request Apr 20, 2016
Provide an example for implementing browser like behavior for console.assert.

This "fixes" #5340 by providing an
alternative to changing Node.js' implemented behavior. Instead, we
document the differences and show how to work around them if
browser like semantics are desired.

Fixes: #5340
PR-URL: #6169
Reviewed-By: Robert Jefe Lindstädt <robert.lindstaedt@gmail.com>
Reviewed-By: Jeff Harris <@techjeffharris>
This was referenced Apr 21, 2016
MylesBorins pushed a commit that referenced this pull request Apr 21, 2016
Provide an example for implementing browser like behavior for console.assert.

This "fixes" #5340 by providing an
alternative to changing Node.js' implemented behavior. Instead, we
document the differences and show how to work around them if
browser like semantics are desired.

Fixes: #5340
PR-URL: #6169
Reviewed-By: Robert Jefe Lindstädt <robert.lindstaedt@gmail.com>
Reviewed-By: Jeff Harris <@techjeffharris>
joelostrowski pushed a commit to joelostrowski/node that referenced this pull request Apr 25, 2016
Provide an example for implementing browser like behavior for console.assert.

This "fixes" nodejs#5340 by providing an
alternative to changing Node.js' implemented behavior. Instead, we
document the differences and show how to work around them if
browser like semantics are desired.

Fixes: nodejs#5340
PR-URL: nodejs#6169
Reviewed-By: Robert Jefe Lindstädt <robert.lindstaedt@gmail.com>
Reviewed-By: Jeff Harris <@techjeffharris>
jasnell added a commit that referenced this pull request Apr 26, 2016
Provide an example for implementing browser like behavior for console.assert.

This "fixes" #5340 by providing an
alternative to changing Node.js' implemented behavior. Instead, we
document the differences and show how to work around them if
browser like semantics are desired.

Fixes: #5340
PR-URL: #6169
Reviewed-By: Robert Jefe Lindstädt <robert.lindstaedt@gmail.com>
Reviewed-By: Jeff Harris <@techjeffharris>
MylesBorins pushed a commit that referenced this pull request May 3, 2016
Provide an example for implementing browser like behavior for console.assert.

This "fixes" #5340 by providing an
alternative to changing Node.js' implemented behavior. Instead, we
document the differences and show how to work around them if
browser like semantics are desired.

Fixes: #5340
PR-URL: #6169
Reviewed-By: Robert Jefe Lindstädt <robert.lindstaedt@gmail.com>
Reviewed-By: Jeff Harris <@techjeffharris>
MylesBorins pushed a commit that referenced this pull request May 6, 2016
Provide an example for implementing browser like behavior for console.assert.

This "fixes" #5340 by providing an
alternative to changing Node.js' implemented behavior. Instead, we
document the differences and show how to work around them if
browser like semantics are desired.

Fixes: #5340
PR-URL: #6169
Reviewed-By: Robert Jefe Lindstädt <robert.lindstaedt@gmail.com>
Reviewed-By: Jeff Harris <@techjeffharris>
MylesBorins pushed a commit that referenced this pull request May 18, 2016
Provide an example for implementing browser like behavior for console.assert.

This "fixes" #5340 by providing an
alternative to changing Node.js' implemented behavior. Instead, we
document the differences and show how to work around them if
browser like semantics are desired.

Fixes: #5340
PR-URL: #6169
Reviewed-By: Robert Jefe Lindstädt <robert.lindstaedt@gmail.com>
Reviewed-By: Jeff Harris <@techjeffharris>
@MylesBorins MylesBorins mentioned this pull request May 18, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
console Issues and PRs related to the console subsystem. doc Issues and PRs related to the documentations.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

console.assert throwing behaviour and Console object
6 participants