Skip to content
This repository has been archived by the owner on Dec 30, 2022. It is now read-only.

fix(metadata): stricter detection of user agent #3184

Merged
merged 2 commits into from
Oct 27, 2021

Conversation

Haroenv
Copy link
Contributor

@Haroenv Haroenv commented Oct 27, 2021

Summary

  1. react native has window, but no navigator
  2. in potentially other environments document might be undefined

see #3145

Result

more robust check of environments.

As these are hard to test, I didn't include a test here per se

1. react native has window, but no navigator
2. in potentially other environments document might be undefined
@codesandbox-ci
Copy link

codesandbox-ci bot commented Oct 27, 2021

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit dc31cd0:

Sandbox Source
react-instantsearch-app Configuration
routing-basic Configuration

@netlify
Copy link

netlify bot commented Oct 27, 2021

✔️ Deploy Preview for react-instantsearch ready!

🔨 Explore the source changes: 42771bf

🔍 Inspect the deploy log: https://app.netlify.com/sites/react-instantsearch/deploys/61795bad2eb41d0007172a44

😎 Browse the preview: https://deploy-preview-3184--react-instantsearch.netlify.app

@Haroenv
Copy link
Contributor Author

Haroenv commented Oct 27, 2021

Thanks for checking @ClaudionorJunior, @johnsoncwb, @LFMAKER, did you confirm this works in your application?

@johnsoncwb
Copy link

johnsoncwb commented Oct 27, 2021

Thanks for checking @ClaudionorJunior, @johnsoncwb, @LFMAKER, did you confirm this works in your application?

@Haroenv Yes, tested and approved.
Screen Shot 2021-10-27 at 11 10 38

@leofolive
Copy link

leofolive commented Oct 27, 2021

@Haroenv This works here, no more crashs.

@netlify
Copy link

netlify bot commented Oct 27, 2021

✔️ Deploy Preview for react-instantsearch ready!

🔨 Explore the source changes: dc31cd0

🔍 Inspect the deploy log: https://app.netlify.com/sites/react-instantsearch/deploys/61795e462d1b6000086d110d

😎 Browse the preview: https://deploy-preview-3184--react-instantsearch.netlify.app

@@ -48,6 +48,17 @@ describe('isMetadataEnabled', () => {
expect(isMetadataEnabled()).toBe(false);
});

it("does not enable when there's no navigator (react native)", () => {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
it("does not enable when there's no navigator (react native)", () => {
it("does not enable when there's no navigator (React Native)", () => {

Comment on lines +6 to +9
typeof window === 'object' &&
typeof window.navigator === 'object' &&
typeof window.navigator.userAgent === 'string' &&
window.navigator.userAgent.includes('Algolia Crawler') &&
Copy link
Member

Choose a reason for hiding this comment

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

Your code works so feel free to merge as is, but this should work no?

Suggested change
typeof window === 'object' &&
typeof window.navigator === 'object' &&
typeof window.navigator.userAgent === 'string' &&
window.navigator.userAgent.includes('Algolia Crawler') &&
window?.navigator?.userAgent?.includes('Algolia Crawler') &&

@Haroenv Haroenv merged commit 994c8ae into master Oct 27, 2021
@Haroenv Haroenv deleted the fix/metadata-react-native branch October 27, 2021 14:25
@johnsoncwb
Copy link

@Haroenv thank you so much! have a nice day.

@Haroenv
Copy link
Contributor Author

Haroenv commented Oct 27, 2021

This has been released in 6.15.0, thanks for your help all!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants