Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

authFlowGenerator fix #880

Merged
merged 3 commits into from
Mar 29, 2019
Merged

authFlowGenerator fix #880

merged 3 commits into from
Mar 29, 2019

Conversation

reldredge71
Copy link
Contributor

No description provided.

@reldredge71 reldredge71 requested a review from OzzieOrca March 28, 2019 20:55
@@ -13,7 +13,7 @@ export const authFlowGenerator = ({
completeAction,
include# = true,
}) => ({
...(include# && {
...((include# && {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think a ternary would read better here. With both && and || there is more to reason about.

Also I'm confused why this is an issue. Doing this in the Chrome console seems to work.
Screen Shot 2019-03-28 at 2 26 21 PM
Maybe Babel compiles it weirdly or JavaScriptCore runs it differently...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does run differently on devices.
This is the issue I found relating to this facebook/react-native#19009

I agree that ternary would be better here. ...(include# ? {...} : {})

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, I will do that.

This error was interesting-- the red screen stated that this was a "performance optimization" and not a "spec complaint" yet it still would not run it.

@reldredge71 reldredge71 merged commit ea7a07e into develop Mar 29, 2019
@reldredge71 reldredge71 deleted the auth-flow-fix branch March 29, 2019 18:45
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants