assert: add strict functionality export#17002

BridgeAR · 2017-11-13T22:50:55Z

To further improve assert this adds a strict export. When used, all functions will use the strict equality instead of a loose one.

At the same time this is a doc only deprecation of the legacy loose mode. It is not intended to remove the old functionality as it is wildly used a lot and removing it does not seem to be a option.
I also improved the assert documentation in general a bit. It now outlines the used rules a bit better and clearly separates loose and strict equality.

Checklist

make -j4 test (UNIX), or vcbuild test (Windows) passes
tests and/or benchmarks are included
documentation is changed or added
commit message follows commit guidelines

Affected core subsystem(s)

doc,assert

Trott

Bold move. I like it in theory, but I'd caution that we are probably never going to be able to get rid of legacy mode, so we will need to support both strict and legacy mode forever... I'm not objecting, but I am being cautious...;.

Trott · 2017-11-14T00:06:29Z

doc/api/assert.md

Micro-nit: Add a blank line before this one?

Trott · 2017-11-14T00:06:52Z

doc/api/assert.md

Micro-nit: comma after mode?

Micro-nit: backticks around strict mode and assert?

Trott · 2017-11-14T00:07:06Z

doc/api/assert.md

Micro-nit: comma before and after for example?

Trott · 2017-11-14T00:08:25Z

doc/api/assert.md

Nit: comma after property?

Trott · 2017-11-14T00:10:31Z

doc/api/assert.md

Nit: Put everything after e.g. and up to the closing . in parentheses.
Nit: Here and several places above, function names should be followed by () I believe. So assert.deepEqual() rather than assert.deepEqual. That said, if there's already a lot of no-parentheses examples in the doc, never mind. :-D

Trott · 2017-11-14T00:11:49Z

doc/api/assert.md

Nit: They're called non-strict, legacy, and loose throughout the document. I'd suggest picking just one of those and sticking with it.
Nit: are often not sufficient might be better expressed as can often have surprising results.

I rephrased it and hope it is clearer now :-)

BridgeAR · 2017-11-14T02:29:34Z

@Trott I addressed all comments. I know it might seem a strong move but we do not have to remove the legacy mode ever and that is also why I only wanted a doc only deprecation.
Having the strict mode will likely improve the situation for many people though.

refack

Quick LGTM

vsemozhetbyt · 2017-11-15T00:53:02Z

doc/api/assert.md

Would it be just #assert_strict_mode? As it is not an assert API part, just a simple heading.

vsemozhetbyt · 2017-11-15T01:04:02Z

doc/api/assert.md

[ strict mode`][]: space instead of a backtick.

joyeecheung · 2017-11-21T21:05:26Z

Ping @BridgeAR this needs a rebase.

BridgeAR · 2017-11-21T23:42:53Z

Rebased

BridgeAR · 2017-11-22T02:54:22Z

CI https://ci.nodejs.org/job/node-test-pull-request/11605/

jasnell · 2017-11-22T17:22:40Z

Needs another rebase :-)

1) Separate all loose and strict functions. 2) Stronger outline the used comparison rules in (not)deepStrictEqual 3) Fix SameValue comparison info

Requireing the strict version will allow to use `assert.equal`, `assert.deepEqual` and there negated counterparts to be used with strict comparison instead of using e.g. `assert.strictEqual`. The API is identical to the regular assert export and only differs in the way that all functions use strict compairson.

BridgeAR · 2017-11-23T09:21:18Z

Rebased plus some tiny documentation improvements while doing so (it is actually the SameValue comparison used by Object.is() and not the Object.is() comparison).

addaleax · 2017-11-28T02:08:06Z

Landed in 06ab6f2, a319e90

1) Separate all loose and strict functions. 2) Stronger outline the used comparison rules in (not)deepStrictEqual 3) Fix SameValue comparison info PR-URL: #17002 Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Vse Mozhet Byt <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>

Requireing the strict version will allow to use `assert.equal`, `assert.deepEqual` and there negated counterparts to be used with strict comparison instead of using e.g. `assert.strictEqual`. The API is identical to the regular assert export and only differs in the way that all functions use strict compairson. PR-URL: #17002 Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Vse Mozhet Byt <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>

MylesBorins · 2017-12-11T22:32:50Z

Should this be backported to v9.x-staging? If yes please follow the guide and raise a backport PR, if not let me know or add the dont-land-on label.

1) Separate all loose and strict functions. 2) Stronger outline the used comparison rules in (not)deepStrictEqual 3) Fix SameValue comparison info PR-URL: nodejs#17002 Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Vse Mozhet Byt <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>

Requireing the strict version will allow to use `assert.equal`, `assert.deepEqual` and there negated counterparts to be used with strict comparison instead of using e.g. `assert.strictEqual`. The API is identical to the regular assert export and only differs in the way that all functions use strict compairson. PR-URL: nodejs#17002 Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Vse Mozhet Byt <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>

BridgeAR · 2018-03-08T16:22:48Z

Backport opened in #19230

1) Separate all loose and strict functions. 2) Stronger outline the used comparison rules in (not)deepStrictEqual 3) Fix SameValue comparison info Backport-PR-URL: #19230 PR-URL: #17002 Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Vse Mozhet Byt <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>

Requireing the strict version will allow to use `assert.equal`, `assert.deepEqual` and there negated counterparts to be used with strict comparison instead of using e.g. `assert.strictEqual`. The API is identical to the regular assert export and only differs in the way that all functions use strict compairson. Backport-PR-URL: #19230 PR-URL: #17002 Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Vse Mozhet Byt <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>

MylesBorins · 2018-07-31T18:26:04Z

Requested backport to 8.x in #19230

Requireing the strict version will allow to use `assert.equal`, `assert.deepEqual` and there negated counterparts to be used with strict comparison instead of using e.g. `assert.strictEqual`. The API is identical to the regular assert export and only differs in the way that all functions use strict compairson. PR-URL: nodejs#17002 Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Vse Mozhet Byt <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>

Requireing the strict version will allow to use `assert.equal`, `assert.deepEqual` and there negated counterparts to be used with strict comparison instead of using e.g. `assert.strictEqual`. The API is identical to the regular assert export and only differs in the way that all functions use strict compairson. Backport-PR-URL: #23223 PR-URL: #17002 Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Vse Mozhet Byt <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>

BridgeAR added assert Issues and PRs related to the assert subsystem. doc Issues and PRs related to the documentations. semver-minor PRs that contain new features and should be released in the next minor version. labels Nov 13, 2017

nodejs-github-bot added the assert Issues and PRs related to the assert subsystem. label Nov 13, 2017

Trott reviewed Nov 14, 2017
View reviewed changes

BridgeAR force-pushed the assert-strict branch from ad10d9f to 3142402Compare November 14, 2017 02:21

refack approved these changes Nov 14, 2017
View reviewed changes

jasnell approved these changes Nov 14, 2017
View reviewed changes

vsemozhetbyt approved these changes Nov 15, 2017
View reviewed changes

addaleax approved these changes Nov 18, 2017
View reviewed changes

BridgeAR force-pushed the assert-strict branch from 3d43614 to 811df7fCompare November 21, 2017 23:41

BridgeAR added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. assert Issues and PRs related to the assert subsystem. and removed assert Issues and PRs related to the assert subsystem. labels Nov 22, 2017

BridgeAR added 2 commits November 23, 2017 07:18

doc: improve assert documentation
37828f9
1) Separate all loose and strict functions. 2) Stronger outline the used comparison rules in (not)deepStrictEqual 3) Fix SameValue comparison info

BridgeAR force-pushed the assert-strict branch from 811df7f to 9692df7Compare November 23, 2017 09:18

addaleax closed this Nov 28, 2017

addaleax removed the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Nov 28, 2017

MylesBorins added the backport-requested-v9.x label Dec 11, 2017

BridgeAR mentioned this pull request Mar 8, 2018
[9.x backport] assert & util PRs #19230
Closed
4 tasks

targos mentioned this pull request Mar 18, 2018
v9.9.0 proposal #19428
Merged

MylesBorins added backported-to-v9.x and removed backport-requested-v9.x labels Mar 20, 2018

MylesBorins added the backport-requested-v8.x label Jul 31, 2018

tniessen added the deprecations Issues and PRs related to deprecations. label Sep 7, 2018

MylesBorins added backported-to-v8.x and removed backport-requested-v8.x labels Oct 31, 2018

BethGriggs mentioned this pull request Nov 12, 2018
v8.13.0 proposal #23974
Merged

beevelop mentioned this pull request Nov 20, 2018
2018-11-20 Version 8.13.0 'Carbon' (LTS) @MylesBorins prepared by @BethGriggs beevelop/docker-android-nodejs#250
Closed

BridgeAR deleted the assert-strict branch April 1, 2019 23:37

lukechilds mentioned this pull request Apr 8, 2019
Expose strict mode functionality browserify/commonjs-assert#41
Merged

Uh oh!

assert: add strict functionality export#17002

assert: add strict functionality export #17002

Uh oh!

Conversation

BridgeAR commented Nov 13, 2017• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Checklist

Affected core subsystem(s)

Uh oh!

Trott left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

BridgeAR commented Nov 14, 2017

Uh oh!

refack left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

joyeecheung commented Nov 21, 2017

Uh oh!

BridgeAR commented Nov 21, 2017

Uh oh!

BridgeAR commented Nov 22, 2017

Uh oh!

jasnell commented Nov 22, 2017

Uh oh!

BridgeAR commented Nov 23, 2017

Uh oh!

addaleax commented Nov 28, 2017

Uh oh!

MylesBorins commented Dec 11, 2017• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

BridgeAR commented Mar 8, 2018

Uh oh!

MylesBorins commented Jul 31, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants

BridgeAR commented Nov 13, 2017•
edited
Loading

MylesBorins commented Dec 11, 2017•
edited
Loading