Conversation
…messages Previously, type errors in generated code would show '_none_, line 1' which provided no useful debugging information. This commit ensures that all generated AST nodes have proper location information that traces back to the original source location. Changes: - Fixed location propagation in CSS module identifier generation - Improved render_variable to preserve location information - Enhanced addLabel to include location in all generated expressions - Ensured render_make_call maintains proper location attributes Now type errors show actual file names and line numbers (e.g., 'test_styles.ml, lines 78-88') instead of unhelpful '_none_' locations.
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
|
New nightly version has been published to the NPM registry: @davesnx/styled-ppx@0.56.1-2b278ac.0. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
This PR is being reviewed by Cursor Bugbot
Details
Your team is on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle for each member of your team.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
| done | ||
|
|
||
| .PHONY: test | ||
| test-promote: build |
There was a problem hiding this comment.
Bug: Duplicate PHONY target declaration for test
The .PHONY: test declaration appears twice in the Makefile (lines 122 and 132), with the second one incorrectly declaring test as phony for the test-promote target. This causes the test-promote target to be incorrectly associated with the phony declaration, potentially breaking Make's dependency tracking. The second declaration should be .PHONY: test-promote instead.
davesnx
force-pushed
the
start-extraction
branch
from
f7f6649 to
58122f0
Compare
There was a problem hiding this comment.
Pull request overview
This PR introduces CSS extraction capabilities for styled-ppx, enabling static CSS file generation. It implements new cx2 and styled2 extensions that leverage css-grammar for parsing and type-checking, replacing the legacy property parsing system. The changes also include improvements to error messages and locations, new runtime APIs (CSS.make, CSS.merge), and support for minification.
Key changes:
- New
cx2,styled2.*,keyframe2, andstyled.global2extensions with static CSS extraction - Migration from
css-property-parsertocss-grammarfor parsing/validation - Static CSS file generation via
styled-ppx.generatedune integration
Reviewed changes
Copilot reviewed 146 out of 372 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/ppx/src/ppx.re | Added cx2/styled2 extensions, integrated css-grammar parser, improved error handling |
| packages/ppx/src/Css_runtime.re | Implemented render_make_call for cx2, refactored to use css-grammar types |
| packages/ppx/src/Css_file.re | New module for CSS accumulation, atomization, and static file generation |
| packages/ppx/src/Property_to_types.re | Maps CSS properties to Css_types modules for interpolation toString calls |
| packages/ppx/src/settings.re | Added --debug and --minify flags |
| packages/ppx/src/generate.re | Added staticComponentStyled2, stylesAndRefObjectWithStyle for new runtime |
| packages/ppx/test/* | Extensive new tests for cx2, minification, error locations |
| packages/ppx/src/dune | Updated dependencies to include css-grammar-parser, murmur2, unix |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
packages/ppx/test/css-support/type-error-interpolation-location.t/input.re
Show resolved
Hide resolved
| ] | ||
| }; | ||
| let m = Format.asprintf("%a", Ppxlib.Pprintast.expression, payload); | ||
| failwith(m) |> ignore; |
There was a problem hiding this comment.
This debug code (failwith(m) |> ignore) should be removed or wrapped in a debug flag check. The |> ignore after failwith is unreachable and suggests this is leftover development code.
| failwith(m) |> ignore; |
| ), | ||
| ] | ||
| | Error(`Invalid_value(reason)) => [ | ||
| | Error(`Invalid_value(_reason)) => [ |
There was a problem hiding this comment.
The _reason parameter is discarded but could provide valuable debugging context. Consider including it in the error message or at least logging it when debug mode is enabled.
There was a problem hiding this comment.
This PR is being reviewed by Cursor Bugbot
Details
Your team is on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle for each member of your team.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
| | Some existing -> | ||
| Printf.eprintf | ||
| "Warning: multiple input files provided. Using '%s', ignoring '%s'\n" | ||
| arg existing |
There was a problem hiding this comment.
Bug: Warning message has swapped argument order
The warning message says "Using '%s', ignoring '%s'" with arguments arg and existing respectively, but the code actually keeps the new arg (via parse (Some arg) ... on line 308) and discards the existing file. The format string arguments are reversed - the message claims to use arg and ignore existing, which is correct for what the code does, but the argument order in the Printf call is backwards making it print the opposite of what's happening.
2 participants
No description provided.