Skip to content

Enforce Canvas children restrictions and update docs#14

Merged
jonobr1 merged 5 commits intomainfrom
13-dom-prevention
Oct 30, 2025
Merged

Enforce Canvas children restrictions and update docs#14
jonobr1 merged 5 commits intomainfrom
13-dom-prevention

Conversation

@jonobr1
Copy link
Owner

@jonobr1 jonobr1 commented Oct 30, 2025

Added runtime validation to warn when DOM elements are used as children of the Canvas component in development mode. Updated documentation in CLAUDE.md and README.md to clarify that only react-two.js components are allowed inside Canvas, with examples of correct and incorrect usage.

Added runtime validation to warn when DOM elements are used as children of the Canvas component in development mode. Updated documentation in CLAUDE.md and README.md to clarify that only react-two.js components are allowed inside Canvas, with examples of correct and incorrect usage.
Repository owner deleted a comment from claude bot Oct 30, 2025
Repository owner deleted a comment from claude bot Oct 30, 2025
@jonobr1 jonobr1 requested a review from Copilot October 30, 2025 19:34
@jonobr1
Copy link
Owner Author

jonobr1 commented Oct 30, 2025

@claude can you review this PR?

@claude
Copy link
Contributor

claude bot commented Oct 30, 2025

Claude encountered an error —— View job


I'll analyze this and get back to you.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds runtime validation to prevent incompatible DOM elements from being used inside the Canvas component, following the pattern established by react-three-fiber. The validation runs in development mode only and warns developers when they attempt to use DOM elements inside Canvas.

  • Implements validateChildren function that recursively checks for DOM elements in Canvas children
  • Adds development-mode warnings when incompatible children are detected
  • Updates documentation in README.md and CLAUDE.md with usage examples and restrictions

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
lib/Provider.tsx Adds validateChildren function and integrates validation into the Provider component (exported as Canvas)
README.md Adds important notice section explaining Canvas children restrictions with correct/incorrect examples
CLAUDE.md Documents Canvas restrictions in Core Components section and adds Common Issues & Solutions entry

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

lib/Provider.tsx Outdated
* Validates that children are compatible with react-two.js Canvas.
* Warns in development mode if DOM elements or incompatible components are found.
*/
function validateChildren(children: React.ReactNode, depth = 0): void {
Copy link

Copilot AI Oct 30, 2025

Choose a reason for hiding this comment

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

The depth parameter is declared but never used in the function. If it's intended for future use (e.g., limiting recursion depth), consider adding a comment explaining this. Otherwise, remove the unused parameter.

Copilot uses AI. Check for mistakes.
Copy link
Owner Author

Choose a reason for hiding this comment

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

@copilot open a new pull request to apply changes based on this feedback

jonobr1 and others added 2 commits October 30, 2025 12:47
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Copy link
Contributor

Copilot AI commented Oct 30, 2025

@jonobr1 I've opened a new pull request, #15, to work on those changes. Once the pull request is ready, I'll request review from you.

Copilot AI and others added 2 commits October 30, 2025 19:53
Co-authored-by: jonobr1 <220033+jonobr1@users.noreply.github.com>
Remove unused depth parameter from validateChildren
@jonobr1 jonobr1 merged commit 729505d into main Oct 30, 2025
1 check passed
@jonobr1 jonobr1 deleted the 13-dom-prevention branch October 30, 2025 21:51
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