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

fix: Implement createComment and nextSibling nodeOps #717

Merged
merged 2 commits into from
Jun 4, 2024

Conversation

garrlker
Copy link
Collaborator

@garrlker garrlker commented Jun 3, 2024

Fixes #706

When a component is set to not render with v-if, Vue will

  1. Remove that component from the dom/scene
  2. Call createComment("v-if")
  3. Place that comment at the same spot as the previous component as a placeholder

Then when that component's v-if is set to true again, Vue

  1. Removes that comment
  2. Runs createElement to re-create that component
  3. Places it back into it's original spot in the dom/scene

We were not implementing v-if so Vue did not have a placeholder comment to keep track of v-if'd objects positions.
When it called createElement since there was no parent, we were placing it at the scene root

I'm wonder if this has not been working the entire time and none of us noticed?

…objects being v-if'd are not lost by Vue's runtime and incorrectly placed in the scene root
@garrlker garrlker requested a review from andretchen0 June 3, 2024 22:30
Copy link

netlify bot commented Jun 3, 2024

Deploy Preview for tresjs-docs ready!

Name Link
🔨 Latest commit c91c237
🔍 Latest deploy log https://app.netlify.com/sites/tresjs-docs/deploys/665f4098adfc8d0008ef9a73
😎 Deploy Preview https://deploy-preview-717--tresjs-docs.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.

@garrlker garrlker changed the title Implement createComment and nextSibling nodeOps Fix: Implement createComment and nextSibling nodeOps Jun 3, 2024
@garrlker garrlker changed the title Fix: Implement createComment and nextSibling nodeOps fix: Implement createComment and nextSibling nodeOps Jun 3, 2024
Copy link
Contributor

@andretchen0 andretchen0 left a comment

Choose a reason for hiding this comment

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

Hey @garrlker ,

Thanks for the fix. Looks good! 👍

As discussed on Discord, I'll leave the TODO up to you.


I've pushed a playground for v-if along with some basic components for showing tests in playground pages.

@garrlker garrlker merged commit 181e472 into main Jun 4, 2024
7 checks passed
@garrlker garrlker deleted the bugfix/706-v-if-component-detaching-from-parenet branch June 4, 2024 23:16
andretchen0 added a commit that referenced this pull request Jun 7, 2024
alvarosabu pushed a commit that referenced this pull request Jun 12, 2024
#726)

* refactor: add 'is' module/tests

* refactor: push predicates down into functions

* refactor: pass TresContext to nodeOps

* refactor: remove predicates/optional chaining, use is/context

* chore: lint

* refactor: use unknown type, not any

* fix: re-add changes from #717

* refactor: use uknown type, not any

* refactor: use uknown type, not any
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Toggle v-if on a Tres component declared in a separate SFC makes it detach from its parent
2 participants