Skip to content

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
@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