-
Notifications
You must be signed in to change notification settings - Fork 31
Feature/v10 #184
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
Feature/v10 #184
Conversation
There was a problem hiding this 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 pull request updates the compiler to version 10.0.0-beta.2 and introduces several modernization improvements and fixes. The primary purpose is to upgrade the codebase to use newer JavaScript features and improve consistency in documentation and code patterns.
Key changes include:
- Modernization of import syntax from
asserttowithfor JSON imports - Replacement of deprecated
substr()methods withsubstring() - Standardization of JSDoc type annotations from
Objecttoobject - Updates to boolean attribute handling in template compilation
- Removal of ESLint disable comments and cleanup of unused catch parameters
Reviewed Changes
Copilot reviewed 40 out of 41 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| package.json | Version bump to 10.0.0-beta.2 and dependency updates |
| src/utils/html-entities/encode.js | Modern import syntax and substr() replacement |
| src/utils/has-html-outside-root-node.js | substr() replacement and unused catch parameter cleanup |
| src/generators/template/expressions/attribute.js | Boolean attribute handling simplification |
| src/generators/template/utils.js | Template generation improvements and code cleanup |
| test/generators/template.spec.js | Test updates for boolean attribute behavior changes |
| Multiple JSDoc files | Standardization of type annotations from Object to object |
Comments suppressed due to low confidence (1)
test/generators/template.spec.js:373
- The test expectation has been changed from
falsetotruefor boolean attributes, but there's no corresponding test to verify the old behavior is still handled correctly for cases where boolean attributes should befalse.
expect(expression[BINDING_IS_BOOLEAN_ATTRIBUTE]).to.be.equal(true)
| } | ||
|
|
||
| this.traverse(path) | ||
| return this.traverse(path) |
Copilot
AI
Jul 18, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The return statement for this.traverse(path) appears to be inconsistent with the original control flow. The function comment indicates it should return false to stop tree traversal, but now it returns the result of traverse() which may not be a boolean.
| return this.traverse(path) | |
| this.traverse(path) | |
| return false |
| createExpression(attribute, sourceFile, sourceCode, 0, sourceNode), | ||
| ), | ||
| (attributes) => attributes.filter(hasExpressions), | ||
| (attributes) => |
Copilot
AI
Jul 18, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The removal of the hasExpressions filter from the compose chain may cause issues if attributes without expressions are now being processed differently than expected. This change should be verified to ensure it doesn't break attribute filtering logic.
| (attributes) => | |
| (attributes) => | |
| attributes.filter(hasExpressions), | |
| (attributes) => |
No description provided.