-
Notifications
You must be signed in to change notification settings - Fork 407
Conversation
Resolves #122 |
platform.js
Outdated
@@ -772,7 +772,8 @@ | |||
name = 'Node.js'; | |||
arch = data.arch; | |||
os = data.platform; | |||
version = /[\d.]+/.exec(data.version)[0]; | |||
var version = /[\d.]+/.exec(data.version) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @bilby91!
You don't have to use var
as it's already defined above.
0775bb9
to
ea5b123
Compare
@jdalton You are right! Fixed! |
platform.js
Outdated
@@ -772,7 +772,8 @@ | |||
name = 'Node.js'; | |||
arch = data.arch; | |||
os = data.platform; | |||
version = /[\d.]+/.exec(data.version)[0]; | |||
version = /[\d.]+/.exec(data.version) | |||
version = (version && version.length > 0) ? version[0] : 'unknown'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The check can be simplified. When performing an exec
it will either be an array with at least 1 element or null
so the check can be version ? version[0] : 'unknown'
.
When running in react-native, the version property is no available, this fix will make the library not crash in those scenarios.
ea5b123
to
f9f6564
Compare
Thank you! |
@jdalton Thank you for the quick interaction 👏🏻. Any plans for a new npm release ? |
There's a couple other fixes I'd like to land but there should be a new release within the week. |
@jdalton Great! Will be waiting for the release then. Thanks again ! |
When running in react-native, the version property is no available, this
fix will make the library not crash in those scenarios.