Skip to content
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

Allow computed objects and arrays without using object or array #42

Closed
5 tasks done
jkomoros opened this issue Jul 7, 2023 · 1 comment
Closed
5 tasks done

Comments

@jkomoros
Copy link
Owner

jkomoros commented Jul 7, 2023

If you want to have an object or array with computed sub-properties, you have to wrap it in either a type:object or type:array. This is annoying, easy to get wrong, and adds an unnessecary level of extra indirection during authoring.

As a pre-processing step before the seed packet has nested seeds unrolled, process the object. Iterate through them, and if we find any with a SeedReference or SeedData property, then replace the parent with a type:object wrapper (and do the same for arrays, too). This should be a pretty simple transformation and handle most cases fine.

(Once we make it so seed references have a seed, not id property, it will make this behavior even more resilient... as long as your sub-objects don't have a type or seed property then it will work as you think.)

  • Make it work for objects
  • Make it work for arrays
  • Document
  • Update type hierarchy to allow objects and arrays that have seeds in them...
  • Update complex examples to use it
jkomoros added a commit that referenced this issue Jul 7, 2023
This makes seed references more obviously seed references.

This was noted as an improvement in #36. It will also make #42 easier to do, because then the only
disallowed key names will be "type" and "seed".
jkomoros added a commit that referenced this issue Jul 8, 2023
…trary nested Array or Object.

Required manually defining a few types to get around a zod recursion issue, but they're simple types.

Part of #42.
jkomoros added a commit that referenced this issue Jul 8, 2023
For some reason the schema doesn't update?

Part of #42.
jkomoros added a commit that referenced this issue Jul 8, 2023
jkomoros added a commit that referenced this issue Jul 8, 2023
Also, make tests for nested use a typescriptt type instead of a seedPacket.parse. This catches errors at authoring time, ahead of testing time. (Before it wasn't possible due to typing limitations,
but I guess I fixed that at some point?)

Part of #42.
jkomoros added a commit that referenced this issue Jul 8, 2023
jkomoros added a commit that referenced this issue Jul 8, 2023
…y inject necessary `array` and `object` seeds during seed packet expansion.

The upshot is that you can now skip including manual `object` or `array` seed types in seed defintions and they will be added for you.

One gotcha is that sub-objects cannot contain the properties `seed` or `type`, otherwise they will be interpreted as a sub_seed. (This was technically always true though).

This is a gnarly commit with a terrifyingly complex expandSeedComputedObjects method. The only reason I have any confidence whatsoever that it works is that tests pass.

Part of #42.
jkomoros added a commit that referenced this issue Jul 8, 2023
Previously we explicitly forbid it, but as of a8c031c it's now explicitly OK.

This actually enables seed packets to pass validation when they use auto-nested objects and arrays.

Part of #42.
jkomoros added a commit that referenced this issue Jul 8, 2023
@jkomoros
Copy link
Owner Author

jkomoros commented Jul 8, 2023

This is done as of cb74ab6

@jkomoros jkomoros closed this as completed Jul 8, 2023
jkomoros added a commit that referenced this issue Jul 8, 2023
If the sub-object only had seed-references, it wouldn't notice it should unroll (it would only unroll if
it was a nested seed).

Added a test to catch this behavior and ensure it works right.

Part of #42. Noticed while working on #37.
jkomoros added a commit that referenced this issue Jul 8, 2023
Schema type checking has started failing with #42 I think, because this problem wasn't detected but should have been.

Part of #36.
jkomoros added a commit that referenced this issue Jul 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant