Ignore linux for version range checks. #2186
Conversation
|
Ha, that was how I originally had it and second guessed myself.
…On Mon, Jan 30, 2017, 19:32 Jesse Ostrander ***@***.***> wrote:
***@***.**** commented on this pull request.
Seems fine to me, could probably use a separate if that more explicitly
defines what is happening.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#2186 (review)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AFk5121nmEceo_PT19ZuT8BwzD-kzmMBks5rXo9DgaJpZM4LyIi_>
.
|
|
I'd defer to @MarshallOfSound before you change it, he might not mind. Just quickly glancing over it, it seems to be an appendage without explanation. |
| @@ -23,7 +23,7 @@ export default class PlatformSpecific extends Component { | |||
| if (process.platform === this.props.platform) { | |||
| if (!this.props.versionRange) return this.props.children; | |||
|
|
|||
| if (semver.validRange(this.props.versionRange) && semver.satisfies(osVersion, this.props.versionRange)) { | |||
| if ((semver.validRange(this.props.versionRange) && semver.satisfies(osVersion, this.props.versionRange)) || (osVersion === null && process.platform === 'linux')) { | |||
There was a problem hiding this comment.
If osVersion is null we should not render regardless of platform. Also this appears to short-circuit the wrong way. The semver.satisfies check will called on null still.
Reverse the "or" order and don't check the platform 👍
Trying to keep this components in a state where they could be published separately
|
This does not fix the problem. As I described in the issue, the problem is mainly at lines 5-6 of the file |
|
@voria Interesting, I had the white screen issue and this fixed it for me. Maybe when I make @MarshallOfSound's suggested changes, it'll fix it for you. |
|
fwiw this patch does fix #2103 for me on git master, thank you! |
Patch fixes issue #2103.