Manager: Fix invalid null assertion #104307

Merged
Sybren A. Stüvel merged 1 commits from abelli/flamenco:invalid-null into main 2024-05-13 16:21:40 +02:00
Contributor

Overview

This patch removes the null check of a condition that evaluates if a string variable is empty.
In JavaScript, empty strings are truthy values compared to null.

Request

That original code already won't work, customBlenderExe != (null || '') is not going to do what it's intended to express. null is a false-y value, and so (null || '') gets shortcut to just '').

That's not something to address in this PR though -- this PR adds a feature, and shouldn't also try and fix existing code. But if you're willing to look into this too, a separate PR that fixes this (which could then be landed before adding this new feature) would be much appreciated.

Originally posted by @dr.sybren in #104306 (comment)

Tests

const customBlenderExe = ''

console.log(customBlenderExe != null)           // true
console.log(customBlenderExe != '')             // false
console.log(customBlenderExe != (null || ''))   // false
### Overview This patch removes the null check of a condition that evaluates if a string variable is empty. In JavaScript, empty strings are truthy values compared to null. ### Request > That original code already won't work, `customBlenderExe != (null || '')` is not going to do what it's intended to express. `null` is a false-y value, and so `(null || '')` gets shortcut to just `''`). > > That's not something to address in this PR though -- this PR adds a feature, and shouldn't also try and fix existing code. But if you're willing to look into this too, a separate PR that fixes this (which could then be landed before adding this new feature) would be much appreciated. > > > _Originally posted by @dr.sybren in https://projects.blender.org/studio/flamenco/pulls/104306#issuecomment-1186952_ ### Tests ```js const customBlenderExe = '' console.log(customBlenderExe != null) // true console.log(customBlenderExe != '') // false console.log(customBlenderExe != (null || '')) // false ```
Mateus Abelli added 1 commit 2024-05-12 15:11:54 +02:00
Remove the null check of a condition that evaluates if a string variable
is empty. In JavaScript, empty strings are truthy values compared to null.
Sybren A. Stüvel approved these changes 2024-05-13 16:19:49 +02:00
Sybren A. Stüvel left a comment
Owner

Thanks for the fix!

Another one for the tests: just console.log(null || '') -- it'll log <empty string>.

When A is falsey, A || B evaluates to B -- in this case the expression evaluates to the empty string. So customBlenderExe != (null || '') is exactly the same as customBlenderExe != ''.

In this case customBlenderExe is just a string, so there is no need to compare with null anyway.

Thanks for the fix! Another one for the tests: just `console.log(null || '')` -- it'll log `<empty string>`. When `A` is falsey, `A || B` evaluates to `B` -- in this case the expression evaluates to the empty string. So `customBlenderExe != (null || '')` is exactly the same as `customBlenderExe != ''`. In this case `customBlenderExe` is just a string, so there is no need to compare with `null` anyway.
Sybren A. Stüvel merged commit 4aa391c679 into main 2024-05-13 16:21:40 +02:00
Sybren A. Stüvel deleted branch invalid-null 2024-05-13 16:21:46 +02:00
Sign in to join this conversation.
No description provided.