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 packages version and ready for pnpm #1333

Merged
merged 5 commits into from
Oct 16, 2023
Merged

Update packages version and ready for pnpm #1333

merged 5 commits into from
Oct 16, 2023

Conversation

Angra974
Copy link
Contributor

@Angra974 Angra974 commented Oct 13, 2023

Description

Using pnpm to install the application is beneficial if you install often packages or if you have a slow network with other benefits. pnpm store the packages in a folder and use them when needed instead of download them again.

Nevertheless, we have a warning with the current installation regarding eslint :

Capture d’écran du 2023-10-13 22-32-48

We need to adjust the dependencies so we can take off this warning.
Before, some people was unable to install with pnpm too because of this problem with eslint.

Fixes #1189

Type of change

Updated the dependencies and their peer dependencies to get no more warning

Only package.json and eslintrc.js was changed by my work.
The other files result in the format and lint of the file with the new version of the modules.

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

Lint after install the current reactplay.
Update the dependencies and the related peer dependencies
Lint after update.
Add configuration in .eslintrc.js to get warnings instead of errors for unused vars.

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce.
=> the basic lint command.

Checklist:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

Screenshots or example output

Capture d’écran du 2023-10-13 23-38-34

video
Capture d’écran du 2023-10-14 11-25-25
Capture d’écran du 2023-10-14 11-27-07
Capture d’écran du 2023-10-14 11-28-13

Remark: Husky commit use Yarn, so ^^'

Notice :
i've updated the .gitignore file to :

  • ignore a test created for cypress that will be placed in cypress/e2e, created by create-react-play if #PR46 is validated by no impact on actual app
  • ignore a file index.json file in the plays folder, created by create-react-play if PR is validated by no impact on actual app ( will be done in some time ).

@netlify
Copy link

netlify bot commented Oct 13, 2023

Deploy Preview for reactplayio ready!

Name Link
🔨 Latest commit 921287c
🔍 Latest deploy log https://app.netlify.com/sites/reactplayio/deploys/652d7ba3f5510a00080ca318
😎 Deploy Preview https://deploy-preview-1333--reactplayio.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@priyankarpal
Copy link
Member

We should upgrade all the dependencies since a few of them have become deprecated. @Angra974

@Angra974
Copy link
Contributor Author

Angra974 commented Oct 14, 2023

We should upgrade all the dependencies since a few of them have become deprecated. @Angra974

Updating all the dependencies means to modify codes inside the application ( change between swiper 9 to 10 ).
Some components needs to be lightly modified.
Some dependencies will not be changed because the plays associated with them won't work ( code-editor for exemple ).
I won't be able to test them with Yarn.
Only pnpm and npm as it is use with the workflow here.
For react-p5, it's the author who depreciate the package. No new version for it.

@Angra974
Copy link
Contributor Author

This one is ready.
Deploy is ok.
Less modifications than expected as some are not update to the last version.
Will come later as they needs update in code and will take more time to be validated.

@Angra974 Angra974 changed the title Update eslint version and configuration for pnpm usage Update packages version and ready for pnpm Oct 14, 2023
Copy link
Member

@priyankarpal priyankarpal left a comment

Choose a reason for hiding this comment

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

Looks Good 🎉

@priyankarpal priyankarpal merged commit 1f31fb2 into main Oct 16, 2023
@priyankarpal
Copy link
Member

so now we need to change the docs according to pnpm such as (pnpm i, pnpm dev)

@priyankarpal priyankarpal deleted the pnpm_eslint branch October 16, 2023 18:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Installation fails often with npm/pnpm because typescript-eslint🐛 and some others errors [Bug report]:
3 participants