Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes a crash that occurred when calling toJSON() on parsed HTML documents due to a circular reference caused by the document property on root nodes. The fix makes the document property non-enumerable while maintaining backward compatibility.
Changes:
- Modified
lib/html/parse-styles.jsto useObject.definePropertywithenumerable: falsefor thedocumentproperty - Added regression test in
test/issue146.jsto verifytoJSON()no longer crashes
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| lib/html/parse-styles.js | Changed document property assignment to use Object.defineProperty with enumerable: false to prevent toJSON serialization issues while maintaining backward compatibility |
| test/issue146.js | Added regression test to verify that calling toJSON() on parsed documents no longer throws an error |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const document = syntax.parse("<style>a {}</style>", { | ||
| from: `/test.html`, | ||
| }); | ||
| document.toJSON(); |
There was a problem hiding this comment.
The test should verify that the document property is still accessible after the fix. This ensures backward compatibility is maintained, as mentioned in the comment on line 170. Consider adding an assertion like expect(document.nodes[0].document).to.equal(document); after calling toJSON() to verify the property still exists and references the correct document.
1 participant
close #146