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

userAgent Bug #281

Merged
5 commits merged into from
Jan 3, 2022
Merged

userAgent Bug #281

5 commits merged into from
Jan 3, 2022

Conversation

echatzief
Copy link
Contributor

Brief Summary of Changes

What does this PR address?

  • Gitub issue (Add reference - #XX)
  • Refactoring
  • New feature
  • Bug fix
  • Adds more tests

Are tests included?

  • Yes
  • No

Reviewer, Please Note:

When you use it for Node.js the API breaks when you define userAgent globally
@patrick-tolosa
Copy link
Contributor

Hey @echatzief - What bug does this PR fix?

@echatzief
Copy link
Contributor Author

Hey @echatzief - What bug does this PR fix?

The module is used at the strapi-provider-upload-cloudinary and the navigator is globally defined. Thus, using the library with node.js the navigator breaks (because it's not defined) and throws error to the above plugin. So, you must define the userAgent inside each function not globally.

@patrick-tolosa
Copy link
Contributor

Thanks, sounds like the right approach.

We'd like to avoid this code duplication though, can you please move the navigator line to a function? getNavigator() (or any other suitable name) instead of copy pasting this line over and over.

Added a function that returns the navigator
@patrick-tolosa
Copy link
Contributor

Almost done here, please move it to the top of the file and we're good to merge.

@patrick-tolosa patrick-tolosa requested a review from a user January 2, 2022 17:06
src/util/browser.js Outdated Show resolved Hide resolved
@echatzief echatzief requested a review from a user January 2, 2022 21:57
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.

2 participants