Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

make interop conditional more clear #322

Merged
merged 1 commit into from
Aug 19, 2017

Conversation

hassanbazzi
Copy link
Member

No description provided.

@@ -9,7 +9,7 @@ else if ('serviceWorker' in navigator && location.protocol === 'https:') {
}


const interopDefault = m => m && m.default || m;
const interopDefault = m => m && m.default ? m.default : m;

Copy link
Member

Choose a reason for hiding this comment

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

This is easy to understand.

👍 LGTM!

Copy link
Member

Choose a reason for hiding this comment

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

but it takes more space :(

Copy link
Member

Choose a reason for hiding this comment

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

true, but easy to understand ;)

@reznord reznord requested a review from lukeed August 18, 2017 09:44
@thangngoc89
Copy link
Collaborator

The way I see it is extractly the same. This is actually common in JS codebases. Try jQuery :D

@developit
Copy link
Member

developit commented Aug 19, 2017

heh yeah they look the same to me, but I am also a masochist

@reznord reznord merged commit 19067c7 into preactjs:master Aug 19, 2017
@lukeed
Copy link
Member

lukeed commented Aug 19, 2017

I'm a fan of the &&|| 😉 so i must be one too

@developit
Copy link
Member

we are mutants

@lukeed
Copy link
Member

lukeed commented Aug 19, 2017

or simulations 💻

@reznord
Copy link
Member

reznord commented Aug 19, 2017

Count me in !! ;)

This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants