Parse-SDK-JS icon indicating copy to clipboard operation
Parse-SDK-JS copied to clipboard

Published 20 hours ago • verified publisher icon parse-community

refactor: bump UUID dep, remove 'deep require' of uuid

Open jpgupta opened this issue 2 years ago • 12 comments

See UUID documentation - https://github.com/uuidjs/uuid#deep-requires-now-deprecated

New Pull Request Checklist

  • [X] I am not disclosing a vulnerability.
  • [X] I am creating this PR in reference to an issue.

Issue Description

The way Parse requires the uuid package is discouraged by the maintainers, and seems to be causing issues with other dependencies in my project.

Related issue: https://github.com/parse-community/Parse-SDK-JS/issues/1491

Approach

This PR updates the uuid dependency fro 3x to 8x, and modifies the way it is imported/required, to use a named import, instead of the deprecated deep require appraoch.

I am not an expert in the nuances of JS import/require approaches so would appreciate some input from someone on this change -it seems to be sensible to me and in line with what the uuid project maintainers recommend (https://github.com/uuidjs/uuid#deep-requires-now-deprecated).

TODOs before merging

  • [ ] Add entry to changelog

jpgupta avatar Jun 06 '22 22:06 jpgupta

Thanks for opening this pull request!

  • ❌ Please edit your post and use the provided template when creating a new pull request. This helps everyone to understand your post better and asks for essential information to quicker review the pull request.

parse-github-assistant[bot] avatar Jun 06 '22 22:06 parse-github-assistant[bot]

I will reformat the title to use the proper commit message syntax.

parse-github-assistant[bot] avatar Jun 06 '22 22:06 parse-github-assistant[bot]

Could you take a look at the failing tests?

mtrezza avatar Jun 10 '22 12:06 mtrezza

@mtrezza Yes I'll take a look this week (For reference / in case anyone can help - it seems to be a permissions related thing? Not sure if people come across this commonly when commiting to this repo for the first time?)

Screenshot 2022-06-12 at 12 44 25

jpgupta avatar Jun 12 '22 11:06 jpgupta

Maybe rebasing this PR on alpha fixes this, see https://github.com/parse-community/Parse-SDK-JS/pull/1495.

mtrezza avatar Jun 12 '22 12:06 mtrezza

@mtrezza I'll give that a go thank you!

jpgupta avatar Jun 20 '22 20:06 jpgupta

It seems the package-lock file still has a conflict

mtrezza avatar Jun 22 '22 20:06 mtrezza

@mtrezza - taking a look

jpgupta avatar Jun 28 '22 23:06 jpgupta

@mtrezza - I'm seeing this message, could you kindly re-run this workflow? Screenshot 2022-07-26 at 23 57 20

jpgupta avatar Jul 26 '22 22:07 jpgupta

Did you recreate the package-lock file? 26,671 additions, 165 deletions looks unusual for this simple dependency upgrade.

mtrezza avatar Jul 26 '22 23:07 mtrezza

@jpgupta Could you use the package-lock file from the alpha branch and do the dependency upgrade again?

mtrezza avatar Jul 28 '22 20:07 mtrezza

@mtrezza sorry for the delay, bunch of things going on - will take a look at this in the next few days

jpgupta avatar Sep 08 '22 19:09 jpgupta

Closing for bot to recreate PR.

dplewis avatar Jan 27 '23 20:01 dplewis