Skip to content
This repository was archived by the owner on Jun 18, 2018. It is now read-only.

Remove lodash/memoize dependency #39

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Remove lodash/memoize dependency #39

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 4 additions & 7 deletions src/BrowserDetector.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,6 @@
import memoize from 'lodash/memoize';

export const isFirefox = memoize(() =>
/firefox/i.test(navigator.userAgent)
);
const _isSafari = Boolean(window.safari);
const _isFirefox = !_isSafari && /firefox/i.test(navigator.userAgent);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do you need the !_isSafari here?

Copy link
Author

Choose a reason for hiding this comment

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

No. It was stupid. For some reason I had the impression that this file was ment to be as optimized as possible, and I assumed that for all iOS-devices out there it would be a tiny bit faster to not have to execute that regex.

It could also be an idea to not use an regexp for this simple task at all:

const _isFirefox = navigator.userAgent.toLowerCase().indexOf('firefox') !== -1;

Copy link
Collaborator

Choose a reason for hiding this comment

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

That also seems like a positive change to me.

Let's just guard these in checks so that this doesn't blow up server rendering!


export const isSafari = memoize(() =>
Boolean(window.safari)
);
export const isFirefox = () => _isFirefox;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is there a good reason why we export functions here instead of just the values?

Copy link
Member

Choose a reason for hiding this comment

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

Probably tried to avoid blowing up server rendering. Should be fine if you add checks for navigator / window and do this eagerly.

export const isSafari = () => _isSafari;