-
-
Notifications
You must be signed in to change notification settings - Fork 93
chore: reworked how the underlying expressions-type system works #1391
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
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1391 +/- ##
=======================================
Coverage 93.70% 93.70%
=======================================
Files 111 111
Lines 4387 4387
Branches 1389 1389
=======================================
Hits 4111 4111
Misses 276 276 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Size Change: 0 B Total Size: 134 kB ℹ️ View Unchanged
|
| { | ||
| "name": "interpolation_type", | ||
| "type": "[\"linear\"] | [\"exponential\", base] | [\"cubic-bezier\", x1, y1, x2, y2]" | ||
| "type": ["[\"linear\"]", "[\"exponential\", base]", "[\"cubic-bezier\", x1, y1, x2, y2]"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this should likely be documented as a array with one syntax enum in it.
Still fairly messy as-is.
Different PR though.
|
I am having a bit of trouble getting the maputnik integration tests to run at all. Sice the dev mode works and the unit tests run, i am not super concearned. |
|
I don't think Maputnik cares about expression syntax, this was added recently to mimic the typescript syntax, mainly for the docs, you should be able to find the relevant PR using blame. |
I don’t think fully correct and complete codegen is realistically possible right now without manual hacks.
This PR adds value by making the type system for expressions more consistent and easier to work with. Documenting things in Markdown works for now, but if we want to support expression codegen in the future - and some of our tools already rely on general codegen - then having a more coherent type system becomes increasingly important. As noted above, this PR doesn’t represent the final state needed for actual codegen, but it is an important stepping stone toward that goal. |
|
Fair enough, thanks for the extra info. |
currently our expressions type system is typescript-ish types (not quite) and mostly documented in MD.
I think documenting this in a structured form has advantages, such as moving the docs to the appropriate place:
Breakage report: