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 #1483

Merged
merged 21 commits into from
Dec 2, 2021
Merged

Update dependencies #1483

merged 21 commits into from
Dec 2, 2021

Conversation

kpuputti
Copy link
Contributor

@kpuputti kpuputti commented Nov 29, 2021

Update several dependencies. See changes to package.json for full info.

@Gnito
Copy link
Contributor

Gnito commented Nov 30, 2021

Couple of comments:

  • dotenv libs should be in sync with Create React App's libs
  • Prettier: I'm not sure if it's worth it since this will a) make it harder for customizers to take upstream update and b) we need to make upstream updates to FTW-hourly & FTW-product

I didn't mention yesterday, but loadable components are also tied with our CRA fork - so, if sharetribe-scripts is not updated at the same time, then some SSR testing is needed to verify that client app versions of those libs are working with the version of loadable stuff we have on sharetribe-scripts.
https://github.com/sharetribe/create-react-app/blob/master/packages/react-scripts/package.json#L33-L35

@kpuputti
Copy link
Contributor Author

kpuputti commented Dec 1, 2021

@Gnito Good points about Prettier. That just introduces conflicts with no real benefits. And customizers can easily update the lib and format if they really want that (although that might introduce conflicts in any case).

I did check to match dotenv with the versions in sharetribe-scripts, but good to know that Loadable should be matches as well. I'll check that.

@kpuputti kpuputti force-pushed the update-dependencies branch 2 times, most recently from 03f4738 to 6ce92dd Compare December 1, 2021 14:24
@kpuputti
Copy link
Contributor Author

kpuputti commented Dec 1, 2021

Now the Prettier and Loadable updates have been removed.

Copy link
Contributor

@Gnito Gnito left a comment

Choose a reason for hiding this comment

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

LGTM.
Note: it might make sense to review the other PR first before merging this to master. Mainly, because, that feature branch is deployed to staging atm.

#1486

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