prismicio / prismic-ts-codegen

A Prismic model-to-TypeScript-type generator.
Apache License 2.0
18 stars 6 forks source link

missing type in automatic type generation of a Slice: id #27

Closed maapteh closed 2 years ago

maapteh commented 2 years ago

While generating auto typings im missing the id, which is in the data but not in its typing for slices.

Versions

Reproduction

None, private repo but screenshots added and should be sufficient

Steps to reproduce

Generate auto typing. For our Slice we get as type:

Screenshot 2022-08-11 at 11 50 18

so no id, while the data we get is:

Screenshot 2022-08-11 at 11 50 43

What is expected?

I just hoped the id: string was part of the generated slice type

What is actually happening?

It is not added, so in my typescript project i cant reference to it. Its wrong in your consumed package:

import {
    CustomTypeModelSlice,
    CustomTypeModelSliceType,
} from "@prismicio/types";

where the definition https://github.com/prismicio/prismic-types/blob/master/src/fields.ts#L797-L817 lacks the id, im not sure why its not there and who ever then used its typings :)

angeloashmore commented 2 years ago

@maapteh Nice catch!

The id property was added to Prismic's API just under two weeks ago, so you caught us between deploying the new property and updating @prismicio/types. 😅

@prismicio/types v0.2.2 was just published and includes the id field for Slices.

You can upgrade with the following command:

npm install @prismicio/types@latest

If that doesn't fix the issue, please let me know and I'll reopen the issue and investigate. Thanks! 🙂

maapteh commented 2 years ago

I can put that dep in my tree, but it should be a dep of the codegen. Not a dev dep in this package :)

@angeloashmore Maybe its an idea to have all these packages in a monorepo so they all align: https://github.com/prismicio/prismic-helpers/blob/master/package.json#L60 internally i now have for example:

❯ yarn why @prismicio/types
├─ @prismicio/client@npm:6.6.3
│  └─ @prismicio/types@npm:0.2.0 (via npm:^0.2.0)
│
├─ @prismicio/helpers@npm:2.3.2
│  └─ @prismicio/types@npm:0.2.0 (via npm:^0.2.0)
│
├─ @prismicio/richtext@npm:2.1.0
│  └─ @prismicio/types@npm:0.1.29 (via npm:^0.1.22)
│
└─ @prismicio/richtext@npm:2.1.1
   └─ @prismicio/types@npm:0.2.0 (via npm:^0.2.0)

Maybe a monorepo makes changes in publishing way more easier to maintain?

angeloashmore commented 2 years ago

Hey @maapteh, we're actually exploring merging our core libraries into one. That would mean combining @prismicio/client, @prismicio/helpers, and @prismicio/types into a unified @prismicio/client package.

I don't think we'll move toward a monorepo in the near future, but merging those core libraries will have a similar effect in simplifying versions and installation.

For now, @prismicio/types is a peer dependency of prismic-ts-codegen. We actually recommend installing @prismicio/types alongside prismic-ts-codegen. This allows you and us to upgrade types independently from prismic-ts-codegen.

From the README and docs:

npm install --save-dev prismic-ts-codegen @prismicio/types

Happy to hear more suggestions if this is causing other problems. 🙂

maapteh commented 2 years ago

@angeloashmore it is typed as a Maybe while in our datamodel it seems to always be a string.

angeloashmore commented 2 years ago

Hey @maapteh,

That's right; id typed as an optional property. Since this property is such a new feature, it is not available on all Prismic repositories yet. To be safe, it is typed as optional to ensure an application's code can handle both cases.

Once the feature is guaranteed to be available in all repositories, we'll promote id to a non-optional field.

If you are rendering your Slices manually and using the ID property to key your Slice components, you can use @prismicio/react's <SliceZone> component instead. The component will automatically use the id property to key the component if available, or fall back to a JSON representation of the Slice in older repositories.

If you are using the ID property for other uses, then I recommend using something like JSON.stringify(slice) as a stable ID if the id property is not available. Using JSON.stringify() is not necessarily efficient for a number a reasons, but it may be an acceptable solution depending on the use case.