a bit of type checking #27097

Tcharl · 2024-08-27T18:09:24Z

Move yo-rc definition to a typescript object.

Further PR will try to narrow the amount of fields filled at each step of generator phases and move the types to their respective generators. Not perfect yet and waiting for advices

contribute to #26114

Please make sure the below checklist is followed for Pull Requests.

All continuous integration tests are green
Tests are added where necessary
The JDL part is updated if necessary
jhipster-online is updated if necessary
Documentation is added/updated where necessary
Coding Rules & Commit Guidelines as per our CONTRIBUTING.md document are followed

When you are still working on the PR, consider converting it to Draft (below reviewers) and adding skip-ci label, you can still see CI build result at your branch.

mshima · 2024-08-28T13:13:11Z

I don't think we should make jhipsterConfig typed.

blueprints will need to convert to any before using.

Tcharl · 2024-08-28T16:00:23Z

This is why I kept the & Record<string, any> at the end of the AbstractJSONGeneratorJhipsterContent.
I'm convinced that knowing all the available and filled options at every step of every generator will improve readability, velocity, stability and keep (once everything done) cli and jdl options aligned.

Please change your mind: it's definitely hard to introduce (or fix) the current codebase for not near full time contributors, and setting type checking would definitely help in that regard (at least).

Finally, i'm also pretty sure that moving these types and default values to their respective generators (i.e. moving JSONGeneratorJhipsterClientContent to generators/client) will help us finding smart modularity tricks (valuable for blueprints)

mshima · 2024-08-28T17:00:17Z

jhipsterConfig and jhipsterConfigWithDefaults are never manually used anymore.

Command has evolved as somewhat source of truth.
https://github.com/mshima/generator-jhipster/blob/f0a8e84e2fbd24ab2f7e9cbe2bcac581004aa671/generators/git/command.ts#L37-L39

A storage scoped config:

allows cli option/argument with choices
allows prompts
allows post processing
is loaded from cli/prompt to the jhipsterConfig
is loaded from jhipsterConfigWithDefaults to the application object.

Probably will allow jdl definitions in the future.

Tcharl · 2024-08-28T17:28:18Z

Thanks for the tips, seems to be a very nice approach

Still, jhipsterConfig* are used in the codebase (via the 'loaded from the cli to jhipster config') so from an implementor point of view using types should help the contributors to understand features more easily (and get IDE code completion), particularly which field is available at which lifecycle step (which confuses even me).
I plan to merge the jdl/JSONGeneratorJhipsterXContent with their <generator>/types.d counterparts in the next commits and use the same types for application: i.e. removing jdl/JSONGeneratorJhipsterAuthenticationContent in favor of base-application/types.d/AuthenticationType to avoid code dupe and enforce consistency.

Doing it step by step just to avoid breaking change and hardcore peer review. Pretty sure the code refactor will lead to breeze and awesome ease to scale again (again ;-)).

Anyway, this PR doesn't harm much and help overall (blueprints should not even need as any), and also contributes to a future simplification of the codebase...

Tcharl · 2024-08-30T15:53:03Z

Is it correct? simplifies a bit the default value code

mshima · 2024-08-31T02:13:49Z

I think we should generate Types from command instead of reworking types see #27133.

Tcharl · 2024-08-31T11:57:02Z

I got it already: command act as the single source of truth.
Still, at on point in time during the generation execution lifecycle:

Commands will be merged with defaults when no answer: here, the objective of this PR in the long term will be to get rid of the 'jhipsterconfigWithDefault' in favor of default values introduced in the respective <generator>/command.ts.
Commands results will be injected in an object representing all the answers of a user for generator prompts: configure part of the command: the goal of this PR is to materialize the resulting typescript object (let's call it GeneratorXConfig) and all its attributes (an object per generator), instead of using jhipsterConfig which is the aggregate of the GeneratorConfigs of all the generators and which should only exist temporary.
These objects will then be enriched with derived properties EnrichedGeneratorXConfig.
These EnrichedGeneratorConfigs are then classified: platforms ones, application ones and entities-related: each generator Config will be classified in one of these three category (PlatformConfig, ApplicationConfig, EntityConfig). -> I'm not sure this step should happen, but I guess that sometimes you'll need an aggregate to produce a full template, your opinion?
Finally, these EnrichedConfigs will be used in preprocess, process and postprocess: here, the developer will definitely be happy to get autocomplete and know at which phase he can have access to which attribute of this config in his template, which is why I started the PR.

I know it's far from being finished yet and that it is quite an adventure to get rid of the current pains related:

Knowing configuration attributes available at each lifecycle phase
Modular generator architecture not centralized on 'big aggregate' objects which would help blueprints
JDL vs Cli gaps and synchronization.

But we have to start from something and this is a small step to start from.

PS: Ah, and BTW, I have a bit of time due to my current situation to contribute in the mid term to this kind of rework. I just don't want to push big stuff with hundreds of conflicts and make it step by step with this objective in mind until it happens

mshima · 2024-08-31T13:00:14Z

generators/spring-cloud/generators/gateway/jdl/jdl-routes-option.spec.ts

Unit test needs a minimum reproduction, if we provide a complete sample it may break with unrelated changes.

mshima · 2024-08-31T13:00:45Z

jdl/converters/json-to-jdl-converter.spec.ts

Unit test needs a minimum reproduction, if we provide a complete sample it may break with unrelated changes.

mshima · 2024-08-31T13:03:27Z

generators/base-application/types.d.ts

These properties are available in app/command.
We should extract from there.

Still, at one point it has to be materialized to an object

mshima · 2024-08-31T13:06:00Z

generators/client/types.d.ts

@@ -10,12 +10,17 @@ type ClientFrameworkType = ['no', 'angular', 'react', 'vue', 'svelte'];

 type ClientFrameworkApplication = OptionWithDerivedProperties<'clientFramework', ClientFrameworkType>;

+export type PersistedClientApplication = {


Persisted types are the ExportStoragePropertiesFromCommand types.

mshima · 2024-08-31T13:06:23Z

generators/client/types.d.ts

@@ -10,12 +10,17 @@ type ClientFrameworkType = ['no', 'angular', 'react', 'vue', 'svelte'];

 type ClientFrameworkApplication = OptionWithDerivedProperties<'clientFramework', ClientFrameworkType>;

+export type PersistedClientApplication = {
+  withAdminUi: boolean;


It’s already available in the bellow type.

mshima · 2024-08-31T13:08:48Z

jdl/converters/types.d.ts

Default types should be dropped from converters and replaced with generics.
We should not try to export or integrate them.

mshima · 2024-08-31T13:12:53Z

generators/base-core/generator.ts

@@ -68,6 +68,7 @@ import { getGradleLibsVersionsProperties } from '../gradle/support/dependabot-gr
 import { dockerPlaceholderGenerator } from '../docker/utils.js';
 import { getConfigWithDefaults } from '../../jdl/index.js';
 import { extractArgumentsFromConfigs } from '../base/internal/command.js';
+import { JSONGeneratorJhipsterContent } from '../../jdl/converters/types.js';


We should not mix jdl with generator types.
To add a type to jhipsterConfig a new type should be created join storage types from every generator.

Tcharl · 2024-08-31T17:52:48Z

Gonna retry with a smaller scope

github-actions bot added the theme: java label Aug 27, 2024

Tcharl force-pushed the typescript-lint branch from adec1ed to 0d764fc Compare August 28, 2024 16:11

github-actions bot added the theme: jhipster-internals label Aug 28, 2024

Tcharl force-pushed the typescript-lint branch from 2e5d5f1 to 863f887 Compare August 29, 2024 15:08

github-actions bot added the theme: front label Aug 29, 2024

Tcharl added 4 commits August 31, 2024 14:00

a bit of type checking

66b2480

enforce some fields to be filled after default config call

aa53433

few constant use instead of plainstring

ba99976

reuse some generator types at converter level

6bd66aa

Tcharl force-pushed the typescript-lint branch from 863f887 to 6bd66aa Compare August 31, 2024 12:06

mshima reviewed Aug 31, 2024

View reviewed changes

Tcharl closed this Aug 31, 2024

Tcharl mentioned this pull request Aug 31, 2024

typechecking on client generator #27134

Closed

6 tasks

mraible added this to the 8.7.1 milestone Sep 21, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

a bit of type checking #27097

a bit of type checking #27097

Tcharl commented Aug 27, 2024 •

edited

Loading

mshima commented Aug 28, 2024

Tcharl commented Aug 28, 2024 •

edited

Loading

mshima commented Aug 28, 2024

Tcharl commented Aug 28, 2024 •

edited

Loading

Tcharl commented Aug 30, 2024

mshima commented Aug 31, 2024

Tcharl commented Aug 31, 2024 •

edited

Loading

mshima Aug 31, 2024

mshima Aug 31, 2024

mshima Aug 31, 2024

Tcharl Aug 31, 2024

mshima Aug 31, 2024

mshima Aug 31, 2024

mshima Aug 31, 2024

mshima Aug 31, 2024

Tcharl commented Aug 31, 2024

		@@ -10,12 +10,17 @@ type ClientFrameworkType = ['no', 'angular', 'react', 'vue', 'svelte'];

		type ClientFrameworkApplication = OptionWithDerivedProperties<'clientFramework', ClientFrameworkType>;

		export type PersistedClientApplication = {

a bit of type checking #27097

a bit of type checking #27097

Conversation

Tcharl commented Aug 27, 2024 • edited Loading

mshima commented Aug 28, 2024

Tcharl commented Aug 28, 2024 • edited Loading

mshima commented Aug 28, 2024

Tcharl commented Aug 28, 2024 • edited Loading

Tcharl commented Aug 30, 2024

mshima commented Aug 31, 2024

Tcharl commented Aug 31, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Tcharl commented Aug 31, 2024

Tcharl commented Aug 27, 2024 •

edited

Loading

Tcharl commented Aug 28, 2024 •

edited

Loading

Tcharl commented Aug 28, 2024 •

edited

Loading

Tcharl commented Aug 31, 2024 •

edited

Loading