generator-jhipster icon indicating copy to clipboard operation
generator-jhipster copied to clipboard

Relationships constants in generators

Open Tcharl opened this issue 9 months ago • 3 comments

The goal being to be able to find in one click within an IDE all the relationship usage within the entire jhipster codebase.

Do you agree with the design? Is so, I'll continue on the entire codebase


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

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.

Tcharl avatar May 08 '24 19:05 Tcharl

Many generators are written in typescript now. In my opinion using variables now is not as useful as it was before. It creates too much overhead.

import { fooType } from '../../jdl/jhipster/index.js';

const { A_CONSTANT } = fooType;

value === A_CONSTANT;

Instead of:

value === 'aConstant'; // validated by typescript (undefined | 'aConstant' | 'anotherConstant')

mshima avatar May 08 '24 20:05 mshima

Agreed that it needs more lines, still, using constants instead of plain strings permits:

  • Folding in IDE ('find usage'), which helps newcomers to better understand the codebase
  • Prevents Typo
  • Ease the migration to proper typescript (constant will be an enum of the class)
  • Ensures proper modules acyclic graph (because we're referencing a constant that is in a 'support' package of another module)
  • Ensures type safety on methods signatures
  • Allows loops over constants of the consts record
  • And for sure a bit more (I hope that typescript will soon implements sealed class like java does)

So IMHO, the benefits overcomes the drawbacks.

Tcharl avatar May 08 '24 21:05 Tcharl

I don’t think we should go this way.

import { RelationshipTypes, ONE_TO_ONE } from '../../entity/support/index.js';
RelationshipTypes[ONE_TO_ONE])

Instead of typescript verified.

'one-to-one'

It’s much more complicated.

mshima avatar Jun 21 '24 00:06 mshima