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(primitive): implement as proxy to avoid breaking references #764

Merged
merged 46 commits into from
Jul 10, 2024

Conversation

andretchen0
Copy link
Contributor

This is the last in a series of PRs to implement primitives in a way that doesn't break references in <script setup> (see #701). With this PR, <primitive> is now implemented as a JS Proxy.

Problem

#701 Under the hood, <primitive /> copies its :object. This breaks references to the object in <script setup>.

Solution

Implement <primitive /> as a JS Proxy.

Playground

http://localhost:5173/issues/701

Problem

#751 Rigged models are squished and do not animate. Bones and animations lose their reference to the model.

Solution

Implement <primitive /> as a JS Proxy.

Playground

http://localhost:5173/models/rigged

Problem

#721 Tres typically removes/disposes in descendants-first order – children are removed before parents. nodeOps.remove sometimes calls a THREE object's dispose method. THREE's dispose implementation does not check for the existence of attributes before trying to delete them – which can result in a user-facing error.

Solution

Try/catch on dispose in nodeOps.remove.

Problem

Cientos' components using primitives under the hood – Sparkles and Contact Shadows – do not work under Tres v4.

Solution

Implement <primitive /> as a JS Proxy.

Playground

http://localhost:5173/issues/701-cientos-v4

closes #751
closes #721
closes #701

Copy link

netlify bot commented Jul 8, 2024

Deploy Preview for tresjs-docs ready!

Name Link
🔨 Latest commit 9fb8cc9
🔍 Latest deploy log https://app.netlify.com/sites/tresjs-docs/deploys/668da913f4040f0008bf1137
😎 Deploy Preview https://deploy-preview-764--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.

@andretchen0 andretchen0 changed the title fix/primitive fix(primitive): implement as proxy to avoid breaking references Jul 8, 2024
@alvarosabu alvarosabu added p5-urgent-bug Fix build-breaking bugs affecting most users, should be released ASAP (priority) needs-documentation labels Jul 9, 2024
@alvarosabu alvarosabu marked this pull request as ready for review July 9, 2024 11:30
Copy link
Member

@alvarosabu alvarosabu left a comment

Choose a reason for hiding this comment

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

Hi @andretchen0 I think I'm gonna pin this PR somewhere just as an example of how a PR should be done, thanks for the incredible amount of work, this is how primitives should have been working fro the beginning but now thanks to you they work.

src/composables/useTresContextProvider/index.ts Outdated Show resolved Hide resolved
src/core/nodeOps.ts Outdated Show resolved Hide resolved
src/core/nodeOps.ts Show resolved Hide resolved
playground/src/pages/advanced/disposal/index.vue Outdated Show resolved Hide resolved
@alvarosabu alvarosabu self-requested a review July 10, 2024 07:48
@alvarosabu alvarosabu merged commit f637bf3 into main Jul 10, 2024
7 checks passed
@alvarosabu alvarosabu deleted the fix/primitive branch July 10, 2024 11:24
@whitespacecode whitespacecode mentioned this pull request Jul 11, 2024
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-documentation p5-urgent-bug Fix build-breaking bugs affecting most users, should be released ASAP (priority)
Projects
Status: Done
2 participants