Skip to content

WIP config parser - #1561

Closed
frolic wants to merge 16 commits into
mainfrom
holic/config-parser
Closed

WIP config parser#1561
frolic wants to merge 16 commits into
mainfrom
holic/config-parser

Conversation

@frolic

@frolic frolic commented Sep 21, 2023

Copy link
Copy Markdown
Member

I started playing around with #994 and at the time was trying to refactor some of our config parsing, but realized I was fighting Zod most of the time. I didn't realize it until well into refactoring that Zod doesn't carry through strong types (i.e. string literals from const types), so you have to both define the Zod type/validation/parsing and the TS transformation.

This seemed like a lot more than we need right now, especially if we feel good about leaning on TS for most of the validation, so I started a new config parser that is pure TS and carries strong types. It's made up of small composable pieces that should hopefully make it easier to maintain, extend, etc.

I'm sure we can extract some common patterns into a Zod-like interface, but we can figure that out later.

TODO:

  • figure out why the union of config root-level tables and namespace-level tables don't intersect properly in some cases
  • figure out what to do about tables with the same name but diff namespaces (this doesn't work well when organizing config output by table key)
  • prototype plugin system with input/output interfaces

@changeset-bot

changeset-bot Bot commented Sep 21, 2023

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: e3363d3

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

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

/** @internal */
export function isTableShapeInput(input: unknown): input is TableShapeInput {
if (!isPlainObject(input)) return false;
if (Object.keys(input).some((key) => !tableInputShapeKeys.includes(key as (typeof tableInputShapeKeys)[number])))

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

we do an isPlainObject check first because Object.keys can have some funky behavior for non-objects:
image

@frolic

frolic commented Sep 29, 2023

Copy link
Copy Markdown
Member Author

took a break from namespaces to play around with plugins + extending types:

https://www.typescriptlang.org/play?ts=4.9.5#code/C4TwDgpgBAysCGwCWBjAggIyQFXNAvFAEQCuSAdsAEwCsAbEVAD7EYjAQDOAzFUQNwAoUJCgAREOXgBbVJhx4ohIp2AAnCgHNGLImw6cBwxTBQALCNPjzcownERyst6CwlTZ6Z3iHHRAaQgQUwsrJSgAb0EoKDUIeAATAHtyABsQKABtAGsALihVDXJNAF18h2QvBUghAF9fEWgANXhUkggQy3hwqJi4xJT0rLyC9S0y2HMumx9BesFBCg41ADN4FGhseAxUiABJcjASYEjoqA8uMHWIAH58wq0hGIu70aLNJ6hsoM6rV4qnNVXFBAsEplZPgA3VrtX7wf7g6zeUQsFptDqIuoLJYQVbXKBbHYQADyxyOwAAPBRyVAIAAPDjkBKcAnbXYHckAPlOzxkl2u+Wpx0yRAunCuGyIJVpDIgTJZD2KUBuUCFwBFYolEClUHyRCMvOkEHuY2Kn2+YNC8HyoLhUJhGKt+TRsMxc2xlFxaw2rKJnA5xx5UAQfvyvRisXiyTSGRyJveE0J7MOx0+81qHuW3s2bK4pOA5KpKZO9MZzN9u39xe54YA9LWoCh4OQAOQnZLBpJgAC0u0hEFSUAABtlVeQvkEkisx+Sh1AAO5IYBmMdZ66cM4hyths59KODWOjigTkBTmfCohbrhSxO5-OFtUiq+GEo5EqctNYvzQADCKRWSCaAACm0mjHoQ4alnKCR-uQAGaPkRY0lB8pQLB8EBsAnIABS7uewCCsWAA0eFJGSxz5OhgH3scSHHJyJExAAlEo3JUZoNGUmqH5zL4OJ4j67GYUGz6vOGe4DDGwzxuM+RJvsxafmcYCgRQnCvOxIEkGB5CZCUX78dmaH-tR5FccWMpliyQnViJuacDuEaRpJQw5GOJ5no+l72TehHkk+PnSih5biRGcZvLJFYKeSnwxBmTkxCq8mcXR6reX6UpvpyeExPk5AQP2ahKRmggrCQ5AoMgKRQFcaicBA7GpZZ0HWSZmiYThZxqn5xwkUxlFtSl3FBvWBLEmIxK5GccTACQajjmqUDwCyzYgF+tYAFQbdEG3GQkECaHKNWqeQO21pmXr4vJwnhigST7Yd5BiXhCSIPAcBqCQlWvBgSRJLszbFRdAk5kSQ0WcFLLXbZt33QdcqORGr0IB9X0EVAAByKQYyQqSpLmqUindD1ylKnIisj73qGjOqQ1Av3-fEp0JcqmPY7j+NEoTRDE-D5BkxTb2o5VUo5bqwafRAQMLIId3kKojZteEtX1ex2Hhs+iMxAAEgOqRJFrEaagKxBmEgRCMRGtSWzEABKf3o6FMS849htOZTwvo2sqT1TbVt4fFUDW3MS0snLqhMb4sttQAdM+Me63jSQx8bGxCKNLOZwlAB6NzR3BgFx-ZMf20kwAp3y4rXOnDZZ3XUC5zLcvwUXfoJ3rycu3KMce9TlU1-Xg8s43zeF-Hpfl135A90LffAAPQ+Lw3ecLEAA

Comment on lines +19 to +20
expectTypeOf(output).toEqualTypeOf(expectedOutput);
expectTypeOf(output).toMatchTypeOf(expectedOutput);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

what's the difference between toEqualTypeOf and toMatchTypeOf?

@frolic

frolic commented Oct 30, 2023

Copy link
Copy Markdown
Member Author

Closing for now. We added a small incremental improvement in #1826 until we can get to the full config overhaul and will likely start fresh when we get to it.

@frolic frolic closed this Oct 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

No open projects

Development

Successfully merging this pull request may close these issues.

2 participants