Skip to content

Conversation

@Bassel17
Copy link
Member

@Bassel17 Bassel17 commented May 5, 2022

What does it do?

  • Allows the compiler.run(configPath, configOptions); function to accept new options from the arguments.
  • Allows us to be able to specify what fileNames we want to compile by providing the path of the files, and allows us to extend or override compiling options.
  • Add incremental compiling before executing CLI commands that need the configuration files in a typescript project : the commands are : strapi console, strapi config:dump, strapi config:restore, strapi admin:create, strapi admin:reset

Why is it needed?

Allow the users to be able to execute CLI commands and succeed just like in a javascript Strapi project

How to test it?

Run the mentioned CLI commands in a Typescript Strapi project, they should behave in the same manner as a javascript strapi project.

docs can be found here: https://docs.strapi.io/developer-docs/latest/developer-resources/cli/CLI.html#strapi-configuration-dump

@codecov
Copy link

codecov bot commented May 5, 2022

Codecov Report

Merging #13245 (a1e584a) into features/typescript (c7b4d13) will decrease coverage by 0.03%.
The diff coverage is n/a.

@@                   Coverage Diff                   @@
##           features/typescript   #13245      +/-   ##
=======================================================
- Coverage                48.23%   48.20%   -0.04%     
=======================================================
  Files                      245      251       +6     
  Lines                     8774     8954     +180     
  Branches                  1955     2008      +53     
=======================================================
+ Hits                      4232     4316      +84     
- Misses                    3732     3820      +88     
- Partials                   810      818       +8     
Flag Coverage Δ
front ?
unit 48.20% <ø> (-0.04%) ⬇️

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

Impacted Files Coverage Δ
...ervices/permission/permissions-manager/sanitize.js 87.71% <ø> (+10.36%) ⬆️
packages/core/admin/server/services/user.js 83.49% <ø> (ø)
...erver/services/schema-builder/component-builder.js 14.03% <ø> (ø)
...er/services/schema-builder/content-type-builder.js 8.73% <ø> (ø)
...pe-builder/server/services/schema-builder/index.js 10.14% <ø> (ø)
...r/server/services/schema-builder/schema-handler.js 3.57% <ø> (ø)
packages/core/strapi/lib/commands/admin-create.js 85.71% <ø> (-1.52%) ⬇️
packages/core/strapi/lib/commands/admin-reset.js 96.29% <ø> (-3.71%) ⬇️
packages/core/upload/server/bootstrap.js 66.66% <ø> (-3.61%) ⬇️
packages/core/utils/lib/hooks.js 89.65% <ø> (ø)
... and 27 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 c250186...a1e584a. Read the comment docs.

@Bassel17 Bassel17 requested review from Convly, Rymakarouf and soupette May 5, 2022 14:39
Copy link
Member

@Convly Convly left a comment

Choose a reason for hiding this comment

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

Nice, some suggestions

module.exports = {
/**
* Default TS -> JS Compilation for Strapi
* @param {string} tsConfigPath
Copy link
Member

Choose a reason for hiding this comment

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

Could you update the jsdoc here?

Copy link
Member

@Convly Convly left a comment

Choose a reason for hiding this comment

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

LGTM

By the way, did you take the opportunity to check the differences in compilation time with and without the incremental option? Both for the first build & for builds following a small change

@Convly
Copy link
Member

Convly commented May 12, 2022

@Bassel17 could you update your branch with features/typescript before we can merge?

It should fix the front unit tests failing

@Bassel17
Copy link
Member Author

Bassel17 commented May 12, 2022

This codecov/project seems to be failing now, which is for tests coverage, what shall we do with that ? @Convly

@Convly
Copy link
Member

Convly commented May 13, 2022

This codecov/project seems to be failing now, which is for tests coverage, what shall we do with that ? @Convly

That's fine for this one, no need to worry about it. Thanks for the update!

Copy link
Member

@Convly Convly left a comment

Choose a reason for hiding this comment

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

LGTM

@Convly Convly merged commit 745c8e8 into features/typescript May 13, 2022
@Convly Convly deleted the typescript/fix-cli branch May 13, 2022 08:56
@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-v4-2-0-fourth-beta-for-typescript-support/18869/1

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.

4 participants