I wanted to ask, if you have ever considered to enable the strict compiler option for the Typescript compiler within the created Angular app.
In my opinion one of the great values of Typescript is a stronger typing, providing documentation and more errors that can be caught at compile time instead of run time. The created Angular app has very unrestricted compiler options and uses many patterns closer to Javascript (many unspecified any types, unspecified return paths from if-statements, ...).
One major problem is, that it is easy to relax the compiler options for your projects, if you feel they are too restrictive. The other way around is quite hard. We want to enable strict compiler options, but there are a lot of errors within the generated JHipster Angular app then.
I would be willing to create a PR adjusting the templates to strict compiler options, but only if you are open to the change in the first place :wink:
Otherwise I fear that we will get quite some merge conflicts in the future, when we'll try to upgrade JHipster.
I also feel that we should improve on this. I suggest creating a component/module targeted PRs to help with easy review/discussion. Once we are good on this front, we can also enable the ESLint rule to keep it under check.
+1 from me also. I actually already started to work on this for account module. I hope I can finish PR for account module today-tomorrow and if this seems OK then we can move on with other modules.
Cool :smile: Can I support you in any way? I don't want to just make smart suggestions and then lean back, but I also don't want to slow you down, if you feel like you have to covered it anyway :wink:
Is there any component where I could start to help you? Or do you want to learn from migrating the account module first?
I made PR for the account module: #10639.
Currently #10639 made the following in the account module:
strict is trueTSLint rule "typedef": [true, "call-signature"]If this is OK (if we agree that this is the way to go) then interested people can do PR-s also in other modules/components.
To avoid parallel work maybe it's good idea to leave comment to this ticket if someone started working with some new module/component cleaning.
ESLint rule @typescript-eslint/no-non-null-assertion. If some other PR also needs this and #10639 is not merged yet then minor merge conflict can occur, but this is easily resolvable I think.I made #10683 to resolve this for audits module.
ok @kaidohallik, forget my other comment. Found it :)
I'm putting a bounty, as it's a lot lot lot of work to achieve this
@wmarques, as you started Angular cleaning in #10383 and made huge reviews while resolving this issue and kept me in the right track then I propose that we share this bounty 50-50. WDYT, is this OK for you?
@kaidohallik Well I really didn't do alot, you made the very big part ! Let's do $100 for me and $400 for you, that's ok ? 馃槃
OK, let's share this way, thanks @wmarques !
Great work @kaidohallik !! Can you also check if current code will work with following typescript configurations that can be enabled in angular v9 with strict flag:
"noImplicitAny": true,
"noImplicitReturns": true,
"noImplicitThis": true,
"noFallthroughCasesInSwitch": true,
"strictNullChecks": true,
@jdubois @pascalgrimaud @deepu105 bounty claimed https://opencollective.com/generator-jhipster/expenses/12559
just approuved @wmarques !
Thanks @vishal423! CI in my fork with these options successful. I raised PR #10998 to add additional rules to tsconfig files.
Bounty claimed: https://opencollective.com/generator-jhipster/expenses/12568
just approuved @kaidohallik :)
@kaidohallik thanks so much for all your hard work!! I'm really sorry that I didn't come true on my support :disappointed: Not gonna make any excuses, I should just be realistic about my capacities...
Anyway, I'm glad you at least got a bounty for this :smile:
Also thanks a lot to @wmarques for all the reviews and suggestions :smiley:
You guys are really awesome that you improve this so quickly!!
@codecholeric, without this issue we wouldn't have strict option enabled now. Thank you for creating this issue and therefore initiating this enhancement!
Most helpful comment
@codecholeric, without this issue we wouldn't have strict option enabled now. Thank you for creating this issue and therefore initiating this enhancement!