Skip to content

Conversation

@Convly
Copy link
Member

@Convly Convly commented Jun 3, 2022

What does it do?

Add a command to the CLI to automatically generate typings based on the application schemas, then load them in the Strapi.Schemas global interface so that they can be used in the whole application.

Why is it needed?

Develop experience improvement to automatically regenerate typings for local schemas (components and content types) and benefit from better typings everywhere in the application.

It'll also be necessary to have this tool when we'll start writing new types for the Strapi APIs, so that the types inference can be as good as possible.

How to test it?

  1. Clone the strapi repository
  2. Navigate to the examples/getstarted application
  3. Run the following command: yarn strapi ts:generate-types
    • --verbose to trigger the verbose mode
    • --silent or -s to trigger the silent mode
    • -o or --out-dir to specify the out directory in which the file will be created
    • -f or --file to specific the filename that will contain the types declarations
  4. A file named schemas.d.ts should have been created
  5. Open this file
  6. It should contain TypeScript types definition for every content type of the application (defined by the user, plugins or Strapi)
  7. It should also contain a declare global expression that extends the Strapi.Schemas interface with all the generated typings

@Convly Convly requested review from Bassel17 and alexandrebodin June 3, 2022 14:53
@Convly Convly changed the title [TS] Schema Types Definitions [TS] Schema Types Generation Jun 3, 2022
@codecov
Copy link

codecov bot commented Jun 3, 2022

Codecov Report

Merging #13460 (704cdd8) into features/typescript (975d5bf) will decrease coverage by 2.10%.
The diff coverage is 79.06%.

@@                   Coverage Diff                   @@
##           features/typescript   #13460      +/-   ##
=======================================================
- Coverage                56.69%   54.59%   -2.11%     
=======================================================
  Files                      958     1224     +266     
  Lines                    21697    31089    +9392     
  Branches                  3562     5645    +2083     
=======================================================
+ Hits                     12302    16972    +4670     
- Misses                    8395    12289    +3894     
- Partials                  1000     1828     +828     
Flag Coverage Δ
front 56.69% <ø> (ø)
unit 49.72% <79.06%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...kages/utils/typescript/lib/utils/resolve-outdir.js 62.50% <ø> (ø)
...s/utils/typescript/lib/generators/schemas/index.js 31.14% <31.14%> (ø)
.../utils/typescript/lib/generators/schemas/schema.js 40.00% <40.00%> (ø)
...s/utils/typescript/lib/generators/schemas/utils.js 97.87% <97.87%> (ø)
packages/utils/typescript/lib/generators/index.js 100.00% <100.00%> (ø)
...ls/typescript/lib/generators/schemas/attributes.js 100.00% <100.00%> (ø)
.../utils/typescript/lib/generators/schemas/global.js 100.00% <100.00%> (ø)
...utils/typescript/lib/generators/schemas/imports.js 100.00% <100.00%> (ø)
packages/utils/typescript/lib/index.js 100.00% <100.00%> (ø)
...itPage/components/GlobalActions/utils/constants.js
... and 262 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 975d5bf...704cdd8. Read the comment docs.

Copy link
Member

@alexandrebodin alexandrebodin left a comment

Choose a reason for hiding this comment

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

A few global comments before digging the code:

  • Can you move this logic to the typescript utils so we just call them from the command ?
  • Why not use typescript to build an ast and dump it to a file ?
    it would avoid having to do all those string manipulations and would be easier to test & safer :)

@Convly Convly force-pushed the typescript/schema-types-generation branch from f683462 to 7b89311 Compare June 8, 2022 08:36
@Convly
Copy link
Member Author

Convly commented Jun 17, 2022

A few global comments before digging the code:

  • Can you move this logic to the typescript utils so we just call them from the command ?
  • Why not use typescript to build an ast and dump it to a file ?
    it would avoid having to do all those string manipulations and would be easier to test & safer :)

Nice insights, it's been implemented in 6c9b500

@Convly Convly requested a review from alexandrebodin June 17, 2022 17:09
@strapi-bot
Copy link

This pull request has been mentioned on Strapi Community Forum. There might be relevant details there:

https://forum.strapi.io/t/typescript-support-has-been-added-to-4-3-0-beta-1/19517/2

@Convly
Copy link
Member Author

Convly commented Jul 6, 2022

  • You will have to update the strapi package versions to be all the same there are still some 4.2.0 in there.

I believe the only package that was still in 4.2.0 was the dependency to @strapi/strapi that have been removed.

  • Missing tests

Yep, indeed.

  • I didn't see a lot of error handling, do you think we should do something about it?

What kind of errors do you see happening? Since we're getting the list of content types directly from a fully-loaded Strapi instance, I assumed any schema misconfiguration would be handled by Strapi validation steps. At first, I had a status column in the summary table as well as warning logs for things failings to build but I never encountered such scenarios and it ended up polluting the table output 🤷

  • If the command is successful it would be good to a least log a few infos

I assumed that the verbose command was here to do that. I imagined that sometimes when piping commands or using it in CI or other tools, you might want to silence completely the command.

@strapi-bot
Copy link

This pull request has been mentioned on Strapi Community Forum. There might be relevant details there:

https://forum.strapi.io/t/strapi-second-beta-release-for-v-4-3-0/20170/1

@strapi-bot
Copy link

This pull request has been mentioned on Strapi Community Forum. There might be relevant details there:

https://forum.strapi.io/t/typescript-support-has-been-added-to-4-3-0-beta-1/19517/5

@SalahAdDin
Copy link
Contributor

Is there any ETA for this project?

@Convly
Copy link
Member Author

Convly commented Jul 12, 2022

Is there any ETA for this project?

This feature will be released along with the TypeScript support feature whose ETA is the end of July.

In the meantime, you can test it using 4.3.0-beta.2 (and above) version

@alexandrebodin
Copy link
Member

  • You will have to update the strapi package versions to be all the same there are still some 4.2.0 in there.

I believe the only package that was still in 4.2.0 was the dependency to @strapi/strapi that have been removed.

  • Missing tests

Yep, indeed.

  • I didn't see a lot of error handling, do you think we should do something about it?

What kind of errors do you see happening? Since we're getting the list of content types directly from a fully-loaded Strapi instance, I assumed any schema misconfiguration would be handled by Strapi validation steps. At first, I had a status column in the summary table as well as warning logs for things failings to build but I never encountered such scenarios and it ended up polluting the table output 🤷

I was more referring to an error in the generation (a usecase we missed for ex) that would break and just display a code error to the user. Would have been nice to wrap som of the generators with try catch to display "We could not generate type because of an unknown errors, please report it to ..."

  • If the command is successful it would be good to a least log a few infos

I assumed that the verbose command was here to do that. I imagined that sometimes when piping commands or using it in CI or other tools, you might want to silence completely the command.

We might want to go the other way around and have a -s silent option but display a minimum of info for the user to know what happened

Copy link
Member

@alexandrebodin alexandrebodin left a comment

Choose a reason for hiding this comment

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

I tested the Date thing and I'm noticing a diff we might want to address.

If I set default: "date-string"

The attribute will be DefaultAttr<"date-string">
But if I put a date obj it will be DefaultAttr

I'm wondering if we should have DefaultAttr In the other case 🤔 that seems a bit inconsistent don't you think ?

@Convly
Copy link
Member Author

Convly commented Jul 21, 2022

One thing we could do is to extract the actual date value from the date instance and convert it to a string literal. But by doing this, we would lose the fact that we need an actual date instance. Another possible solution to keep consistency is to willingly use "Date" instead of the actual date string for default date-string values?
I personally don't have preferences 🤷

@Convly Convly merged commit 0c9eabe into features/typescript Jul 25, 2022
@Convly Convly deleted the typescript/schema-types-generation branch July 25, 2022 06:56
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

Successfully merging this pull request may close these issues.

5 participants