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

Improved validation regular expressions to support specific attribute names #356

Merged
merged 17 commits into from
Jan 3, 2025

Conversation

gonft
Copy link
Contributor

@gonft gonft commented Dec 31, 2024

Description

improve attribute name validation regex to support directives

Allow validation of directive-style attributes like ':class' and '@scroll.window' by updating the attribute name validation pattern.

Type of Change

  • 🛠️ Bug fix

Ready Checklist

  • I've read the Contribution Guide.
  • In case this PR changes one of the core packages, I've modified the respective CHANGELOG.md file using
    the semantic_changelog format.
  • I updated/added relevant documentation (doc comments with ///).
  • I added myself to the AUTHORS file (optional, if you want to).

gonft added 2 commits January 1, 2025 00:17
Allow validation of directive-style attributes like ':class' and '@scroll.window' by updating the attribute name validation pattern.
Add test cases to verify DomValidator accepts attribute:
- ':' prefix for binding attributes (e.g. :class)
- '@' prefix for event handlers (e.g. @scroll.window)
@gonft gonft requested a review from schultek as a code owner December 31, 2024 15:32
Copy link

docs-page bot commented Dec 31, 2024

To view this pull requests documentation preview, visit the following URL:

docs.page/schultek/jaspr~356

Documentation is deployed and generated using docs.page.

Copy link

github-actions bot commented Dec 31, 2024

Package Version Report

The following packages have been updated:
jaspr : 0.16.3 -> 0.16.4
jaspr_builder : 0.16.3 -> 0.16.4
jaspr_cli : 0.16.3 -> 0.16.4
jaspr_lints : 0.1.2 -> 0.2.0
jaspr_serverpod : 0.4.0 -> 0.5.0
jaspr_test : 0.16.3 -> 0.16.4

gonft added 4 commits January 1, 2025 00:58
- Implement custom GatherUsedImportedElementsVisitor class to replace removed analyzer functionality
- Remove unused import from import_assist.dart
- Replace deprecated 'enclosingElement' with 'enclosingElement3'
- Add curly braces to if statement in component_assist.dart
- Bump `serverpod` to `2.3.0` and `jaspr` to `0.16.3`
- Update CHANGELOG.md to reflect version 0.5.0 changes
- Bump `serverpod`, `serverpod_auth_client`, and `serverpod_client` to `2.3.0` across multiple files.
- Ensure compatibility with the latest version of the serverpod packages.
Copy link

codecov bot commented Jan 2, 2025

Codecov Report

Attention: Patch coverage is 87.50000% with 1 line in your changes missing coverage. Please review.

Project coverage is 75.86%. Comparing base (89c4bba) to head (064026d).

Files with missing lines Patch % Lines
..._builder/lib/src/styles/styles_module_builder.dart 75.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #356      +/-   ##
==========================================
+ Coverage   75.80%   75.86%   +0.05%     
==========================================
  Files         129      129              
  Lines        5481     5486       +5     
==========================================
+ Hits         4155     4162       +7     
+ Misses       1326     1324       -2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

- Update generated files to reflect the renaming of _Modules to Modules and improved class structure using 'serverpod generate' command.
packages/jaspr/CHANGELOG.md Outdated Show resolved Hide resolved
packages/jaspr_lints/CHANGELOG.md Outdated Show resolved Hide resolved
packages/jaspr_serverpod/CHANGELOG.md Outdated Show resolved Hide resolved
packages/jaspr_serverpod/pubspec.yaml Outdated Show resolved Hide resolved
packages/jaspr_serverpod/pubspec.yaml Outdated Show resolved Hide resolved
packages/jaspr_serverpod/CHANGELOG.md Outdated Show resolved Hide resolved
packages/jaspr_lints/lib/src/assists/import_assist.dart Outdated Show resolved Hide resolved
Copy link
Owner

@schultek schultek left a comment

Choose a reason for hiding this comment

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

The fix itself looks good to me, thanks.

gonft added 4 commits January 2, 2025 23:08
- Update CHANGELOG.md files to reflect the latest changes and versioning.
- Mark jaspr and jaspr_lints as unreleased patches.
- Adjust jaspr_serverpod to indicate an unreleased minor version with a bump to serverpod 2.3.0.
- Revert changes to import retrieval process to maintain compatibility with analyzer ^6.5.0
- compilationUnit.libraryImports requires analyzer >=6.10.0 which is above our current constraint
CI automatically manages version numbers, reverting manual version change.
@gonft
Copy link
Contributor Author

gonft commented Jan 3, 2025

The fix itself looks good to me, thanks.

Hi @schultek Kilian melos analyze is not passing, can I re-commit with an improved version?

gonft added 3 commits January 3, 2025 10:54
- Keep the constraint at the minor level
- Fix GatherUsedImportedElementsVisitor instantiation
- Remove unused import 'package:analyzer/src/error/imports_verifier.dart'
- Update import retrieval to use the library's compilation unit for better accuracy
- Replace direct access to libraryImports and libraryExports with compilationUnit properties
- Improve compatibility with the analyzer package by ensuring correct import processing
- Remove lint warnings by properly handling nullable types and type casting
@schultek
Copy link
Owner

schultek commented Jan 3, 2025

Its fine if the analysis is not passing for the other packages (builder and lints). You don't need to fix them.

@schultek
Copy link
Owner

schultek commented Jan 3, 2025

I've fixed the issues in main and pulled it into the pr.

Copy link
Owner

@schultek schultek left a comment

Choose a reason for hiding this comment

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

LGTM

@schultek schultek merged commit f7566cd into schultek:main Jan 3, 2025
3 checks passed
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.

2 participants