Fix bugs and add CSS feature support by rafaelcaricio · Pull Request #49 · rafaelcaricio/gradient-parser

rafaelcaricio · 2026-02-27T08:15:32Z

High-priority bug fixes:

Fix stringify visit_angular always emitting 'deg' (now preserves rad/grad/turn)
Fix stringify visit_default-radial missing 'at' keyword for explicit at-positions
Replace deprecated String.substr() with substring()
Validate hex color to only accept 3, 4, 6, or 8 digit lengths
Fix license headers (BSD -> MIT) to match package.json/README
Fix start script to use Python 3 (python -m http.server)

New CSS feature support:

Add conic-gradient() and repeating-conic-gradient() parsing and stringification
Add grad and turn angle units (alongside existing deg and rad)
Add rem, vw, vh, vmin, vmax, ch, ex length units for color stop positions
Add CSS Color Level 4 space-separated color syntax (rgb(255 0 0 / 0.5), hsl(120 60% 70% / 0.5))
Support dual color stop positions (e.g. red 10% 30%)

All 144 tests pass (95 existing + 49 new).

https://claude.ai/code/session_01HYURCvRuSARp2j8Qi5nP83

High-priority bug fixes: - Fix stringify visit_angular always emitting 'deg' (now preserves rad/grad/turn) - Fix stringify visit_default-radial missing 'at' keyword for explicit at-positions - Replace deprecated String.substr() with substring() - Validate hex color to only accept 3, 4, 6, or 8 digit lengths - Fix license headers (BSD -> MIT) to match package.json/README - Fix start script to use Python 3 (python -m http.server) New CSS feature support: - Add conic-gradient() and repeating-conic-gradient() parsing and stringification - Add grad and turn angle units (alongside existing deg and rad) - Add rem, vw, vh, vmin, vmax, ch, ex length units for color stop positions - Add CSS Color Level 4 space-separated color syntax (rgb(255 0 0 / 0.5), hsl(120 60% 70% / 0.5)) - Support dual color stop positions (e.g. red 10% 30%) All 144 tests pass (95 existing + 49 new). https://claude.ai/code/session_01HYURCvRuSARp2j8Qi5nP83

https://claude.ai/code/session_01HYURCvRuSARp2j8Qi5nP83

- Update README: document all AST types (hsl, hsla, var, calc, conic), document stringify API, update examples with new angular unit field - Add TypeScript type definitions (index.d.ts) for full IDE support - Add pretest script so tests auto-build before running - Add ESM module support via build/esm.mjs with package.json exports map - Update minimum engine version from >=0.10.0 to >=18.0.0 - Add more descriptive package keywords Note: CI node version update (drop 16, add 22) requires workflows permission and should be applied separately. https://claude.ai/code/session_01HYURCvRuSARp2j8Qi5nP83

Copilot

Pull request overview

This pull request adds significant new CSS gradient parsing capabilities and fixes several bugs in the gradient-parser library. The PR adds support for conic gradients (a CSS feature not previously supported), additional angle units (grad, turn), new length units (rem, vw, vh, vmin, vmax, ch, ex), CSS Color Level 4 space-separated color syntax, and dual color stop positions. It also fixes critical bugs including angle unit preservation in stringify, the missing 'at' keyword in radial gradients, deprecated String.substr() usage, hex color validation, and license header corrections. The PR adds 49 new tests bringing the total to 144 passing tests.

Changes:

Add conic-gradient() and repeating-conic-gradient() support with parsing and stringification
Fix critical bugs: angle unit preservation, radial gradient 'at' keyword, substr() deprecation, hex validation
Add CSS Color Level 4 space-separated syntax support for rgb/rgba/hsl/hsla
Add dual color stop positions, new angle units (grad, turn), and viewport-relative length units
Update license headers from BSD to MIT, add ESM module support, TypeScript definitions, and bump Node.js requirement to >=18.0.0

Reviewed changes

Copilot reviewed 10 out of 12 changed files in this pull request and generated 11 comments.

Show a summary per file

File	Description
lib/parser.js	Core parsing logic: adds conic gradient support, new angle/length units, CSS Color Level 4 syntax, dual color stops, hex validation, and fixes deprecated substr()
lib/stringify.js	Core stringification: adds conic gradient visitors, new length unit visitors, angle unit preservation, dual color stop support, and fixes 'at' keyword bug
spec/parser.spec.js	Test coverage: 49 new tests for conic gradients, angle units, length units, CSS Color Level 4, dual positions, and hex validation
spec/stringify.spec.js	Test coverage: round-trip tests for all new features including angle units, conic gradients, length units, and dual positions
package.json	Metadata updates: ESM support, Node.js >=18.0.0, python3 start script, additional keywords, TypeScript types
package-lock.json	Version bump to 1.1.1 with esbuild dependencies
index.d.ts	New TypeScript definitions file with complete type coverage for all gradient types, color stops, and nodes
build.js	Build system: adds ESM bundle generation alongside existing Node.js and web bundles
build/node.js	Generated Node.js bundle with all parser and stringify changes
build/web.js	Generated web bundle with all parser and stringify changes
build/esm.mjs	New ESM module bundle for modern JavaScript import syntax
README.md	Documentation updates: ESM import examples, API clarification, AST type documentation, and feature descriptions

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-27T10:35:30Z

lib/parser.js

+      var values = [r, g, b];
+      if (scan(tokens.comma)) {
+        values.push(matchNumber());
+        return { type: baseType === 'rgba' ? 'rgba' : 'rgba', value: values };


The conditional expression always returns 'rgba' regardless of the condition. When using legacy comma-separated rgb() syntax with 4 values, this should return 'rgba', but the ternary operator evaluates to 'rgba' in both cases. This should likely be: baseType === 'rgba' ? 'rgba' : 'rgb' to properly handle rgb(r, g, b, a) by converting it to rgba.

Suggested change

return { type: baseType === 'rgba' ? 'rgba' : 'rgba', value: values };

return { type: baseType === 'rgba' ? 'rgba' : 'rgb', value: values };

Copilot · 2026-02-27T10:35:30Z

lib/parser.js


+  function matchRGBValues(baseType) {
+    var r = matchNumber();
+    var lookaheadCache = input;


The variable lookaheadCache is declared but never used. It was likely intended for backtracking if the parse fails, but the logic doesn't restore input from this cache. Consider removing this unused variable.

Suggested change

var lookaheadCache = input;

Copilot · 2026-02-27T10:35:31Z

build/node.js

+      var values = [r, g, b];
+      if (scan(tokens.comma)) {
+        values.push(matchNumber());
+        return { type: baseType === 'rgba' ? 'rgba' : 'rgba', value: values };


The conditional expression always returns 'rgba' regardless of the condition. When using legacy comma-separated rgb() syntax with 4 values, this should return 'rgba', but the ternary operator evaluates to 'rgba' in both cases. This should likely be: baseType === 'rgba' ? 'rgba' : 'rgb' to properly handle rgb(r, g, b, a) by converting it to rgba.

Suggested change

return { type: baseType === 'rgba' ? 'rgba' : 'rgba', value: values };

return { type: baseType === 'rgba' ? 'rgba' : 'rgb', value: values };

Copilot · 2026-02-27T10:35:31Z

build/esm.mjs

+      var values = [r, g, b];
+      if (scan(tokens.comma)) {
+        values.push(matchNumber());
+        return { type: baseType === 'rgba' ? 'rgba' : 'rgba', value: values };


The conditional expression always returns 'rgba' regardless of the condition. When using legacy comma-separated rgb() syntax with 4 values, this should return 'rgba', but the ternary operator evaluates to 'rgba' in both cases. This should likely be: baseType === 'rgba' ? 'rgba' : 'rgb' to properly handle rgb(r, g, b, a) by converting it to rgba.

Suggested change

return { type: baseType === 'rgba' ? 'rgba' : 'rgba', value: values };

return { type: baseType === 'rgba' ? 'rgba' : 'rgb', value: values };

Copilot · 2026-02-27T10:35:31Z

build/web.js


+  function matchRGBValues(baseType) {
+    var r = matchNumber();
+    var lookaheadCache = input;


The variable lookaheadCache is declared but never used. It was likely intended for backtracking if the parse fails, but the logic doesn't restore input from this cache. Consider removing this unused variable.

Suggested change

var lookaheadCache = input;

Copilot · 2026-02-27T10:35:32Z

build/node.js


+  function matchRGBValues(baseType) {
+    var r = matchNumber();
+    var lookaheadCache = input;


The variable lookaheadCache is declared but never used. It was likely intended for backtracking if the parse fails, but the logic doesn't restore input from this cache. Consider removing this unused variable.

Suggested change

var lookaheadCache = input;

Copilot · 2026-02-27T10:35:32Z

build/esm.mjs

+
+  function matchRGBValues(baseType) {
+    var r = matchNumber();
+    var lookaheadCache = input;


The variable lookaheadCache is declared but never used. It was likely intended for backtracking if the parse fails, but the logic doesn't restore input from this cache. Consider removing this unused variable.

Suggested change

var lookaheadCache = input;

Copilot · 2026-02-27T10:35:32Z

package.json

  },
  "engines": {
-    "node": ">=0.10.0"
+    "node": ">=18.0.0"


The Node.js version requirement has been increased from ">=0.10.0" to ">=18.0.0", which is a major breaking change. This change appears to be justified by the introduction of ESM support and the use of esbuild, but such a significant version jump should be carefully documented in the PR description and potentially in a migration guide. Users on older Node.js versions will be unable to upgrade to this version of the package.

Suggested change

"node": ">=18.0.0"

"node": ">=0.10.0"

Copilot · 2026-02-27T10:35:33Z

spec/parser.spec.js

+    it('should still parse legacy comma-separated rgb', function() {
+      var ast = gradients.parse('linear-gradient(rgb(255, 0, 128), blue)');
+      expect(ast[0].colorStops[0].type).to.equal('rgb');
+      expect(ast[0].colorStops[0].value).to.eql(['255', '0', '128']);
+    });
+
+    it('should still parse legacy comma-separated hsl', function() {
+      var ast = gradients.parse('linear-gradient(hsl(120, 60%, 70%), blue)');
+      expect(ast[0].colorStops[0].type).to.equal('hsl');
+      expect(ast[0].colorStops[0].value).to.eql(['120', '60', '70']);
+    });
+  });


Missing test coverage for the edge case where rgb() is called with 4 comma-separated values (e.g., rgb(255, 0, 0, 0.5)). According to the CSS spec, rgb() with 4 values should be treated as rgba(). Adding a test for this case would help catch the bug in the ternary operator at line 394 of lib/parser.js.

Copilot · 2026-02-27T10:35:33Z

package.json

+  },
  "scripts": {
    "test": "mocha spec/**/*.js",
+    "pretest": "npm run build",


The pretest script runs the build before every test execution. This means developers need to wait for the build to complete before running tests, which can slow down the development feedback loop. Consider whether this is necessary or if it would be better to have developers run the build manually when needed, especially during iterative development.

Suggested change

"pretest": "npm run build",

claude added 3 commits February 26, 2026 23:11

chore: update package-lock.json after dependency install

2f1f5f2

https://claude.ai/code/session_01HYURCvRuSARp2j8Qi5nP83

rafaelcaricio requested a review from Copilot February 27, 2026 10:28

Copilot started reviewing on behalf of rafaelcaricio February 27, 2026 10:29 View session

Copilot AI reviewed Feb 27, 2026

View reviewed changes

rafaelcaricio closed this Feb 27, 2026

rafaelcaricio deleted the claude/review-project-gaps-qnGll branch February 27, 2026 13:55

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix bugs and add CSS feature support#49

Fix bugs and add CSS feature support#49
rafaelcaricio wants to merge 3 commits intomasterfrom
claude/review-project-gaps-qnGll

rafaelcaricio commented Feb 27, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Feb 27, 2026

Uh oh!

Copilot AI Feb 27, 2026

Uh oh!

Copilot AI Feb 27, 2026

Uh oh!

Copilot AI Feb 27, 2026

Uh oh!

Copilot AI Feb 27, 2026

Uh oh!

Copilot AI Feb 27, 2026

Uh oh!

Copilot AI Feb 27, 2026

Uh oh!

Copilot AI Feb 27, 2026

Uh oh!

Copilot AI Feb 27, 2026

Uh oh!

Copilot AI Feb 27, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	return { type: baseType === 'rgba' ? 'rgba' : 'rgba', value: values };
	return { type: baseType === 'rgba' ? 'rgba' : 'rgb', value: values };

Conversation

rafaelcaricio commented Feb 27, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants