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

Update dependencies to the latest versions #85

Merged
merged 4 commits into from May 17, 2021
Merged

Update dependencies to the latest versions #85

merged 4 commits into from May 17, 2021

Conversation

yevdyko
Copy link
Collaborator

@yevdyko yevdyko commented May 14, 2021

This MR updates dependencies to the latest versions and removes p5 from the peer dependencies.

What was changed?

  • Update p5 version to the latest 1.3.1.
  • Update dependencies to the latest versions.
  • Remove p5 from peer dependencies.

@jamesrweb
Copy link
Collaborator

jamesrweb commented May 14, 2021

@yevdyko Do we need the dependency in both dependencies and peer dependencies?

Also, does the .x syntax work? Never seen this before and if it does, can you give a quick explainer for future reference?

@yevdyko
Copy link
Collaborator Author

yevdyko commented May 14, 2021

@jamesrweb I think you are right about p5 as a dependency and we should remove it from the peer dependencies as it makes no sense.

As for the 1.x syntax, it's similar to ^1.0.4. See more details here: Using semantic versioning to specify update types your package can accept

Maybe, we need to add the minimum version for React as a peer dependency based on the used features in the library.

@jamesrweb
Copy link
Collaborator

jamesrweb commented May 14, 2021

@jamesrweb I think you are right about p5 as a dependency and we should remove it from the peer dependencies as it makes no sense.

As for the 1.x syntax, it's similar to ^1.0.4. See more details here: Using semantic versioning to specify update types your package can accept

Maybe, we need to add the minimum version for React as a peer dependency based on the used features in the library.

After some thinking, I remembered that it was actually added there by me as a peer dependency because of an issue I resolved where users installing the library has a p5 is not defined or a related error appearing (this happened because @and-who removed the peer dependency previously) and so I re-added the peer dependency in #71. Can you test locally using npm link to see if the install throws such an error again now? Maybe safer to revert it to exist to stop a repeat of the issues in #70, not sure, what do you think?

I would prefer using the ^x.x.x syntax since the other packages are like that, what do you think, should we revert or update all, if so, is there a benefit or just a preference? Happy either way but would like to keep things consistent.

@yevdyko
Copy link
Collaborator Author

yevdyko commented May 16, 2021

After some thinking, I remembered that it was actually added there by me as a peer dependency because of an issue I resolved where users installing the library has a p5 is not defined or a related error appearing (this happened because @and-who removed the peer dependency previously) and so I re-added the peer dependency in #71. Can you test locally using npm link to see if the install throws such an error again now? Maybe safer to revert it to exist to stop a repeat of the issues in #70, not sure, what do you think?

I removed the p5 from peer dependencies and double-checked in the demo application locally using link that everything works properly until we have p5 as a dependency. I think this was the main reason why users were having some problems. You added p5 to dependencies on 23 Sep 2020, but it has not been published in npm and yarn until 14 Mar 2021. As a result, users were forced to install p5 separately as it was in the peer dependencies but not installed while the package installation. So we can now safely remove it from the peer dependencies and leave it as a dependency only.

I would prefer using the ^x.x.x syntax since the other packages are like that, what do you think, should we revert or update all, if so, is there a benefit or just a preference? Happy either way but would like to keep things consistent.

The only reason to use 1.x was to have p5 as a peer dependency. I thought it would be more obvious which versions are acceptable, but once we move away from that, we can use the common ^x.x.x. syntax.

@jamesrweb jamesrweb merged commit 240b224 into P5-wrapper:master May 17, 2021
@jamesrweb
Copy link
Collaborator

Merged the current PR, for follow up topics we can open a new one @yevdyko, great work so far, appreciate the contributions! 👌🏻

@yevdyko yevdyko deleted the chore/update-dependencies branch May 17, 2021 07:46
jamesrweb pushed a commit that referenced this pull request Aug 15, 2022
* Update p5 version to the latest 1.x

* Update dependencies to the latest versions

* Remove p5 from peer dependencies

* Update dependencies since opening PR
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.

None yet

2 participants