Skip to content

Conversation

@4ndv
Copy link

@4ndv 4ndv commented Nov 12, 2025

Summary

Was working on another change in noUnknownProperty, noticed that composes property is already skipped with cssModules enabled in parser config

The query is Ast<CssGenericProperty>, and when cssModules is enabled this property is parsed as CssComposesProperty, excluding it from the query

I think there is no need to allow composes property when css modules parsing is disabled

Prior PR when the check was added: #3217

Test Plan

Added separate test cases with and without cssModules enabled

Docs

Not sure if any is required, but if so, please tell me and I'll update the website

@changeset-bot
Copy link

changeset-bot bot commented Nov 12, 2025

🦋 Changeset detected

Latest commit: 672fa69

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 13 packages
Name Type
@biomejs/biome Patch
@biomejs/cli-win32-x64 Patch
@biomejs/cli-win32-arm64 Patch
@biomejs/cli-darwin-x64 Patch
@biomejs/cli-darwin-arm64 Patch
@biomejs/cli-linux-x64 Patch
@biomejs/cli-linux-arm64 Patch
@biomejs/cli-linux-x64-musl Patch
@biomejs/cli-linux-arm64-musl Patch
@biomejs/wasm-web Patch
@biomejs/wasm-bundler Patch
@biomejs/wasm-nodejs Patch
@biomejs/backend-jsonrpc Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-actions github-actions bot added A-Linter Area: linter L-CSS Language: CSS labels Nov 12, 2025
@4ndv 4ndv changed the title fix(noUnknownProperty): remove explicit ignore for composes in noUnknownProperty fix(noUnknownProperty): remove explicit ignore for composes Nov 12, 2025
@4ndv 4ndv force-pushed the no-unknown-property-composes branch from 60ae11d to 672fa69 Compare November 12, 2025 11:31
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 12, 2025

Walkthrough

The PR modifies the noUnknownProperty linting rule to conditionally handle the composes CSS property. Previously, composes was always excluded from unknown-property checks. Now, it's only ignored when CSS Modules are enabled in parser settings. Test files were reorganised accordingly: composition examples were moved from valid to invalid test files, and a new valid test file was created with CSS Modules configuration enabled. A changeset documents the behaviour change.

Suggested reviewers

  • ematipico
  • dyc3

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description check ✅ Passed The description clearly explains the rationale for the change, references prior work, and outlines the test plan and documentation considerations.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Title check ✅ Passed The title precisely describes the main change: removing explicit ignore for the 'composes' property in noUnknownProperty lint rule.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@4ndv
Copy link
Author

4ndv commented Nov 12, 2025

Forcepushed with different commit name, previous one was too long, fyi

@codspeed-hq
Copy link

codspeed-hq bot commented Nov 12, 2025

CodSpeed Performance Report

Merging #8083 will not alter performance

Comparing 4ndv:no-unknown-property-composes (672fa69) with main (f102661)

Summary

✅ 29 untouched
⏩ 126 skipped1

Footnotes

  1. 126 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@4ndv
Copy link
Author

4ndv commented Nov 12, 2025

Digged into failed tests, now it applies both parser and linter errors for the same line

Is there any way to skip linting lines with parser errors already applied? Removed condition did exactly that, but it looks a bit hacky to me

@ematipico
Copy link
Member

I believe the issue is that now the rule isn't skipping compose at all.

What you want to do is to extract the CssFileSource using

let source_type = ctx.source_type::<CssFileSource>();

However, we don't save the CssModules information anywhere inside the CssFileSource. You'll have to implement it from scratch. As for now, we have Variant::Standard. We could add a new variant.

@4ndv
Copy link
Author

4ndv commented Nov 13, 2025

I'll close it for now, will have another go after different PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-Linter Area: linter L-CSS Language: CSS

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants