The Wayback Machine - https://web.archive.org/web/20220525073850/https://github.com/ionic-team/ionic-framework/pull/24254
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

feat(react): add setupIonicReact function #24254

Merged
merged 4 commits into from Nov 22, 2021
Merged

feat(react): add setupIonicReact function #24254

merged 4 commits into from Nov 22, 2021

Conversation

liamdebeasi
Copy link
Member

@liamdebeasi liamdebeasi commented Nov 22, 2021

Pull request checklist

Please check if your PR fulfills the following requirements:

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been reviewed and added / updated if needed (for bug fixes / features)
  • Build (npm run build) was run locally and any changes were pushed
  • Lint (npm run lint) has passed locally and any fixes were made for failures

Pull request type

Please check the type of change your PR introduces:

  • Bugfix
  • Feature
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • Documentation content changes
  • Other (please describe):

What is the current behavior?

Issue Number: resolves #24139

With the Custom Elements build, we now need to call initialize(): https://github.com/ionic-team/ionic-framework/blob/next/packages/react/src/components/index.ts#L202. The problem is that this call also sets up the Ionic config. Given that Ionic config is not designed to be reactive, subsequent calls to setupConfig had no effect.

What is the new behavior?

  • After discussing with the team, we feel the best path forward is a (small) breaking change. Developers will be required to add setupIonicReact to their App.tsx/App.jsx file. This function will call initialize and allow developers to pass in a config, so it can be used instead of setupConfig.
  • We explored non-breaking change options, but we think this change is the best option long term.
  • I also noticed that setupConfig was being exported from Angular/React/Vue packages when it should not be. Developers should be going through the main entry points and not calling setupConfig on their own. If they need to eject from the normal process, they should call initialize instead.

Does this introduce a breaking change?

  • Yes
  • No

Other information

Migration:

  1. Import setupIonicReact from '@ionic/react';
  2. In App.tsx, call setupIonicReact();. Config should be passed here instead of setupConfig.
  3. Remove any calls to setupConfig.

@github-actions github-actions bot added package: angular package: react package: vue labels Nov 22, 2021
@amandaejohnston
Copy link
Contributor

@amandaejohnston amandaejohnston commented Nov 22, 2021

We'll also need to update the docs when this is merged: https://beta.ionicframework.com/docs/react/config

Copy link
Contributor

@amandaejohnston amandaejohnston left a comment

LGTM other than the BREAKING.md comment 👍 There's also updating the starter apps and docs, but those are other repos so we're good here otherwise.

BREAKING.md Show resolved Hide resolved
@liamdebeasi
Copy link
Member Author

@liamdebeasi liamdebeasi commented Nov 22, 2021

Starters have been updated in the ionic6 branch: https://github.com/ionic-team/starters/compare/ionic6?expand=1
Docs have been updated in to the react-config branch: https://github.com/ionic-team/ionic-docs/compare/react-config?expand=1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package: angular package: react package: vue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants