feat: Improve code to check values for null or undefined.

[nojiritakeshi]

Is your feature request related to a problem? Please describe. First of all, I apologize for my poor English. This is a code improvement proposal, not a feature improvement.

Describe the solution you'd like I think the following code section can be substituted with the code I propose. value === null || value === undefined My Proposal: value == null By using the 'equivalence operator', it should be possible to inspect only null and undefined together.

Describe alternatives you've considered Existing Code value === null || value === undefined My Proposal: value == null

Additional context

[nojiritakeshi]

If the points I make are good ones, may I make a pull request?

[SukkaW]

If the points I make are good ones, may I make a pull request?

Currently, the eslint rules used by the repo are enforcing value === null || value === undefined. This is to prevent an edge case of document.all:

document.all === undefined
// false
document.all === null
// false
document.all == null
// true

[tobySolutions]

If the points I make are good ones, may I make a pull request?

Currently, the eslint rules used by the repo are enforcing value === null || value === undefined. This is to prevent an edge case of document.all:

document.all === undefined
// false
document.all === null
// false
document.all == null
// true

Hmm, interesting. Thanks Sukka!

[SukkaW]

Hmm, interesting. Thanks Sukka!

I am considering re-visit this. If == null is fast enough, we should disable that eslint rule (since nobody uses docuemnt.all now).

[tobySolutions]

Hmm, interesting. Thanks Sukka!

I am considering re-visit this. If == null is fast enough, we should disable that eslint rule (since nobody uses docuemnt.all now).

I was gonna implement this fix in one of my PRs where ai was upgrading dependencies. I think I sort of lost track of it then.

[tobySolutions]

Hmm, interesting. Thanks Sukka!

I am considering re-visit this. If == null is fast enough, we should disable that eslint rule (since nobody uses docuemnt.all now).

So, @nojiritakeshi can handle it then?

[SukkaW]

I am considering re-visit this. If == null is fast enough, we should disable that eslint rule (since nobody uses docuemnt.all now).

So, @nojiritakeshi can handle it then?

We will need to do some benchmarks on this, to see if changing to == null is really worth it.

[nojiritakeshi]

We will need to do some benchmarks on this, to see if changing to == null is really worth it.

Would you like me to join you in checking?

[SukkaW]

We will need to do some benchmarks on this, to see if changing to == null is really worth it.

Would you like me to join you in checking?

Sure, of course!

[tobySolutions]

We will need to do some benchmarks on this, to see if changing to == null is really worth it.

Would you like me to join you in checking?

Sure, of course!

Awesome!

[nojiritakeshi]

By the way, I modified the code of block.ts and measured the speed with block.test.tsx with the following results.

Before correction:

case: should ignore null, undefined, false: 0.404 case: should ignore null, undefined, false: 0.0321 case: should patch block: 0.123 case: should patch block: 0.0238 case: should patch nested blocks: 0.0860

After correction:

case: should ignore null, undefined, false: 0.362 case: should ignore null, undefined, false: 0.0353 case: should patch block: 0.201 case: should patch block: 0.0281 case: should patch nested blocks: 0.0545

More research may be needed, but there may not be much to worry about in terms of speed.(?)

[aidenybai]

Will be updated in a PR for a bigger change soon, closing