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

bug: new useIonModal hook for Ionic React renders component outside tree #23516

Closed
babycourageous opened this issue Jun 26, 2021 · 18 comments
Closed
Labels
package: react @ionic/react package type: bug a confirmed bug report
Milestone

Comments

@babycourageous
Copy link
Contributor

Bug Report

Ionic version:

[ ] 4.x
[x ] 5.x
[ ] 6.x

Current behavior:
Just in case this behavior is unintended, the new useIonModal hook renders the modal component passed to present() outside of the react tree as a sibling of the root node. Because of this, the component used has no access to any app-wide context that wraps the main "App" component that the calling component belongs to.

Expected behavior:
Render the component for the modal inside the root react tree.

Steps to reproduce:

Follow the Ionic docs for rendering/presenting a React modal using useIonModal() hook.

Related code:

Here's a screenshot where you see the component that the hook renders next to the component tree instead of inside it (either as a child or sibling of <App />)
Screen Shot 2021-06-25 at 7 27 22 PM

Other information:

In case this is the intended behavior for the useIonModal hook - this may be an opportunity to mention in the docs when deciding whether to use the original <IonModal> wrapping component or the hook. Perhaps that the hook is meant for modals that don't need access to app-wide data and for those and other complex use-cases continue to use the <IonModal> component.
I would be happy to PR to the docs if needed!
Thanks

Ionic info:

Ionic:

   Ionic CLI       : 5.4.16 (/Users/rene/.npm/_npx/23301/lib/node_modules/ionic)
   Ionic Framework : @ionic/react 5.6.10

Capacitor:

   Capacitor CLI   : 2.4.6
   @capacitor/core : 2.4.7

Utility:

   cordova-res : not installed
   native-run  : not installed

System:

   NodeJS : v12.16.3 (/Users/rene/.nvm/versions/node/v12.16.3/bin/node)
   npm    : 6.14.4
   OS     : macOS Mojave
@ionitron-bot ionitron-bot bot added the triage label Jun 26, 2021
@babycourageous babycourageous changed the title bug: new overlay hook for modal renders component outside tree bug: new useIonModal hook for Ionic React renders component outside tree Jun 26, 2021
@liamdebeasi
Copy link
Contributor

Thanks for the issue. Could you reproduce this issue in an Ionic starter app and provide a link to the repo?

@liamdebeasi liamdebeasi added the ionitron: needs reproduction a code reproduction is needed from the issue author label Jun 28, 2021
@ionitron-bot
Copy link

ionitron-bot bot commented Jun 28, 2021

Thanks for the issue! This issue has been labeled as needs reproduction. This label is added to issues that need a code reproduction.

Please reproduce this issue in an Ionic starter application and provide a way for us to access it (GitHub repo, StackBlitz, etc). Without a reliable code reproduction, it is unlikely we will be able to resolve the issue, leading to it being closed.

If you have already provided a code snippet and are seeing this message, it is likely that the code snippet was not enough for our team to reproduce the issue.

For a guide on how to create a good reproduction, see our Contributing Guide.

@babycourageous
Copy link
Contributor Author

Hi Liam

Absolutely, here ya go!
https://github.com/babycourageous/ionic-react-modal-hook-issue

@liamdebeasi
Copy link
Contributor

Thanks, and sorry for the delay. I am able to reproduce this behavior. I am checking with the team to see if this the intended behavior.

@liamdebeasi liamdebeasi removed the ionitron: needs reproduction a code reproduction is needed from the issue author label Jul 16, 2021
@babycourageous
Copy link
Contributor Author

No prob at all @liamdebeasi ! Thanks for looking into it.

@liamdebeasi
Copy link
Contributor

Heard back from the other team members and can confirm that overlays should not behave this way. No ETA on a fix yet, but it looks to be similar to what we did in Ionic Vue to ensure overlays are rendered within the correct app context: https://github.com/ionic-team/ionic-framework/blob/main/packages/vue/src/components/IonApp.ts

@liamdebeasi liamdebeasi added package: react @ionic/react package type: bug a confirmed bug report labels Aug 9, 2021
@babycourageous
Copy link
Contributor Author

Thanks for the update @liamdebeasi !

@DRiFTy17
Copy link

@liamdebeasi has there been any progress/update on this? We're also running into this issue... Thanks!

@piotr-cz
Copy link

@elylucas

After fix #f3e492c VSCode complains:

'IonApp' cannot be used as a JSX component.
Its instance type 'IonApp' is not a valid JSX element.
Type 'IonApp' is missing the following properties from type 'Component<any, any>': state, props, context, setState, forceUpdate

I'm using vitejs + preact

When I added missing props, error went away.
This might be specific to my build setup

@liamdebeasi
Copy link
Contributor

@piotr-cz Can you provide a reproduction in an Ionic starter app?

@piotr-cz
Copy link

@liamdebeasi
Copy link
Contributor

liamdebeasi commented Oct 29, 2021

Hmm, this might be VSCode getting confused (see below). I am not getting any build errors nor do I get this error when using React. This warning pops up when using IonTabButton as well.

Are you getting this error in any other editors?

@liamdebeasi
Copy link
Contributor

liamdebeasi commented Oct 29, 2021

Looks to be an issue with Preact: preactjs/preact#2748

I'll keep digging and see if there's anything Ionic can do to work around this.

@liamdebeasi
Copy link
Contributor

@piotr-cz Can you give this dev build a shot and let me know if it fixes the issue?

npm install @ionic/[email protected]

@piotr-cz
Copy link

piotr-cz commented Nov 3, 2021

@liamdebeasi I've fixed the issue with hard-wiring paths to react in tsconfig.json as described here: preactjs/preact#2150 (comment)

// tsconfig.json
{
  "compilerOptions": {
    "skipLibCheck": false,
    "baseUrl": ".",
    "paths": {
      "react": ["node_modules/preact/compat"],
      "react-dom": ["node_modules/preact/compat"]
    }
  }
}

@piotr-cz
Copy link

piotr-cz commented Nov 3, 2021

@liamdebeasi
Thanks, #24132 solved the issue in VSCode, however I'm unable to build.
I'll post details in referenced PR

@liamdebeasi
Copy link
Contributor

Thanks for the issue. The original issue here was resolved via f3e492c. The Preact issue has been resolved via #24132, and a fix will be available in an upcoming release of Ionic 6.

@liamdebeasi liamdebeasi modified the milestones: 6.0.0, 6.0.0-rc.3 Nov 3, 2021
@ionitron-bot
Copy link

ionitron-bot bot commented Dec 3, 2021

Thanks for the issue! This issue is being locked to prevent comments that are not relevant to the original issue. If this is still an issue with the latest version of Ionic, please create a new issue and ensure the template is fully filled out.

@ionitron-bot ionitron-bot bot locked and limited conversation to collaborators Dec 3, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
package: react @ionic/react package type: bug a confirmed bug report
Projects
None yet
Development

No branches or pull requests

4 participants