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

Upgrade to polaris 11 and react 18 #72

Closed
wants to merge 5 commits into from
Closed

Conversation

pnmcosta
Copy link

@pnmcosta pnmcosta commented Jun 27, 2023

I have a hard dependency on this package and have to upgrade both polaris and react to latest on my app, which breaks. This PR is a work in progress and aims to fix that. So far have been able to:

  • Bump all dependencies to the latest
  • Update markup (and tests) to use legacy components and refactored where needed.
  • Successfully yarn build *had to change the build verify assertion - probably due to build on different runners.

I could not test with Jest, hoping the CI pipeline will throw any issues to resolve.

@pnmcosta
Copy link
Author

I have signed the CLA!

@pnmcosta pnmcosta changed the title WIP upgrade to polaris 11 and react 18 Upgrade to polaris 11 and react 18 Jul 12, 2023
@husseyexplores
Copy link

Thank you for this PR.
Someone, please merge this... Much needed!

@mathiusj
Copy link
Contributor

mathiusj commented Jul 19, 2023

@pnmcosta Have approved the workflow run, looks like there are still some issues to resolve.

LMK if you run into any problems resolving them and I can look into it :)

@pnmcosta
Copy link
Author

Hi @mathiusj thank you so much for this. I am currently on holiday and will return next week, but just quickly scanning the details of the error, seems to be something related to linting and its config. Is this something you could help me with?

I'm also not clear what the Changelog issue is, am I meant to have a separate tool installed locally to generate the changelog or some sort of manual process?

Thank you for you help

@mathiusj
Copy link
Contributor

Hi @mathiusj thank you so much for this. I am currently on holiday and will return next week, but just quickly scanning the details of the error, seems to be something related to linting and its config. Is this something you could help me with?

I'm also not clear what the Changelog issue is, am I meant to have a separate tool installed locally to generate the changelog or some sort of manual process?

Thank you for you help

@pnmcosta I have grabbed your forked repo and merged it with a branch in the main repo, and will continue the progress there while you're away, following with this pull request. I'll update the changelog and fix lint errors and request reviews internally to get this going ASAP. Enjoy your vacation!! 🙏 we got this for ya 🥂

You should be able to close this one now 😄

@pnmcosta
Copy link
Author

Hi @mathiusj thank you for taking my work and making sure the repo is updated!

I don't see any mention of it on https://github.com/Shopify/discount-app-components/blob/main/CHANGELOG.md can you confirm this now works with React 18 and Polaris 11?

@pnmcosta pnmcosta mentioned this pull request Jul 24, 2023
@pnmcosta pnmcosta closed this Jul 24, 2023
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.

3 participants