Skip to content

Fix lightness-notation crash with "number" option and single-digit percentage #8661

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.

Sign up for GitHub

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

Merged
ybiquitous merged 3 commits into from
Jul 9, 2025
+60 −2
Merged

Fix lightness-notation crash with "number" option and single-digit percentage #8661

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
66989c6
Fix `lightness-notation` crash with `"number"` option and single-digi…
ybiquitous Jul 7, 2025
1ce911e
Fix `%` replacement
ybiquitous Jul 7, 2025
c026856
Merge branch 'main' into issue-8659
ybiquitous Jul 7, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/major-seals-bathe.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"stylelint": patch
---

Fixed: `lightness-notation` crash with `"number"` option and single-digit percentage
39 changes: 39 additions & 0 deletions lib/rules/lightness-notation/__tests__/index.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,19 @@ testRule({
},
],
},
{
code: 'a { color: oklab(0 0.1 241) }',
fixed: 'a { color: oklab(0% 0.1 241) }',
fix: {
range: [17, 18],
text: '0%',
},
message: messages.expected('0', '0%'),
line: 1,
column: 18,
endLine: 1,
endColumn: 19,
},
],
});

Expand Down Expand Up @@ -423,5 +436,31 @@ testRule({
},
],
},
{
code: 'a { color: oklab(5% -100% 0.4) }',
fixed: 'a { color: oklab(0.05 -100% 0.4) }',
fix: {
range: [17, 19],
text: '0.05',
},
message: messages.expected('5%', '0.05'),
line: 1,
column: 18,
endLine: 1,
endColumn: 20,
},
{
code: 'a { color: oklab(0% -100% 0.4) }',
fixed: 'a { color: oklab(0 -100% 0.4) }',
fix: {
range: [18, 19],
text: '',
},
message: messages.expected('0%', '0'),
line: 1,
column: 18,
endLine: 1,
endColumn: 20,
},
],
});
9 changes: 8 additions & 1 deletion lib/rules/lightness-notation/index.cjs
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 changes: 8 additions & 1 deletion lib/rules/lightness-notation/index.mjs
View file
Open in desktop
Copy link
Member

@romainmenke romainmenke Jul 9, 2025

Choose a reason for hiding this comment

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

[Question] Do we want to proactively fix issues with scientific notation? e.g. oklab(1e3% -100% 0.4)

When the input is scientific notation we need to count the digits differently.
When the output is scientific notation we can't trim trailing zero's.

This seems to work:

/**
 * @param {number} num
 * @param {string} value
 * @returns {string}
 */
function roundToNumberOfDigits(num, value) {
	if (num === 0) return '0';

	let precision = 1;

	if (value.includes('e')) {
		let meaningfulDigits = value.split('e', 1)[0] ?? '';

		precision = meaningfulDigits.replace('.', '').length;
	} else {
		precision = value.replace('%', '').replace('.', '').length;
	}

	if (precision === 0) return `${num}`;

	const rounded = num.toPrecision(precision);

	if (rounded.includes('e')) return rounded;

	return rounded.replace(/0+$/, ''); // trim trailing zeros
}

But I think such inputs are extremely rare given the small numeric values that are in range for these functions.

Copy link
Member Author

@ybiquitous ybiquitous Jul 9, 2025

Choose a reason for hiding this comment

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

Oh, nice catch. I also think it's a rare case like 1e3%, so I'd like to merge this as is.

But I agree with supporting such values with scientific notation in a follow-up PR. 👍🏼

I probably guess we should add a utility to parse such number/percentage values. 🤔

Copy link
Member

@romainmenke romainmenke Jul 9, 2025

Choose a reason for hiding this comment

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

A utility with some unit tests would be nice indeed :)
Then we don't need to do exhaustive testing in lint rules.

Original file line number Diff line number Diff line change
Expand Up @@ -139,9 +139,16 @@
/**
* @param {number} num
* @param {string} value
* @returns {string}
*/
function roundToNumberOfDigits(num, value) {
return num.toPrecision(value.length - 2);
if (num === 0) return '0';

const precision = value.replace('%', '').replace('.', '').length;

if (precision === 0) return `${num}`;

return num.toPrecision(precision).replace(/0+$/, ''); // trim trailing zeros
}

/**
Expand Down