aurelia / cli

The Aurelia 1 command line tool. Use the CLI to create projects, scaffold components, and bundle your app for release.
MIT License
407 stars 133 forks source link

[Suggestion] Drop .ext and code duplication in js+ts files #1119

Open michaelw85 opened 5 years ago

michaelw85 commented 5 years ago

I'm submitting a suggestion to improve cli maintainability

1) The CLI skeletons contains .ext files to be able to flexibly change the output to either JS or TS based on choices made by the developer setting up a new project.

Although this is a great concept it has some drawbacks.

2) When we want to add a proper TS file with typings to a skeleton we have a JS and TS variant in a skeleton. This results in duplicate code and maintenance, it easy to forgot one or the other.

Suggestion If https://github.com/aurelia/cli/pull/1067 is completed a transpiler is introduced. We could change all skeletons to a TS setup, this would give the TS users properly typed files and for the JS users we could simply transpile these files. This would remove duplicate code and would provide a normal development experience in the IDE.

3cp commented 5 years ago

All skeleton files are never IDE lint ready or even valid code, because there is a lot preprocess directive @if in those files, some directives broke js/ts syntax, but they will be processed to generate valid code.

For example, a json file with those if comments appeared broken in linter or IDE. There is not much you can do with it.

They are not runable code, let alone to be unit testable. They are templates (half cooked files).

michaelw85 commented 5 years ago

In my opinion the CLI is an essential tool for new Aurelia users. Having a broken tool will result in people having a negative experience with Aurelia. Best case scenario they either invest time and jump to gitter/discourse/github and try to fix it. Worst case they are so annoyed they immediately stop and spread their experience with others. Where I'm going with this, to grow the community this thing should be super stable.

TL&DR I think we should test skeleton files, most importantly the vital parts like tasks.

The JSON files could be created via code in a composition way instead of using the current custom templating setup. Although I think this is super nifty a simple if/else in code would suffice and would make it developer and IDE friendly and as a bonus unit testable, it would be incredibly easy (especially if you would use jest snapshot testing).

3cp commented 5 years ago

The old version of cli generates files through code, it was massive to maintain, and a huge barrier for new comers to contribute. All those manual code were not tested.

The new descriptive-skeleton uses descriptive content and file name to lower the learning curve, also cut down code base significantly. The new generic descriptive-skeleton logic has almost-full unit test coverage. So far, this change has been positive.

I don't understand how you test template files, unless it's in a template language the IDE and linter understands. But it's indeed possible to build preprocess syntax support in vscode and some linter.

3cp commented 5 years ago

I agree jest snapshot could be super useful on template testing. But we need to convert the test code from jasmine to jest.

michaelw85 commented 5 years ago

For the json files your experience is that the current way works better, that is valid reasoning to keep it as-is of course. I would like to make an example of what I have in mind, just a POC. if that's ok with you (this could just be a confirmation for myself that it indeed does not work very well).

My intent for this github "issue" was to discuss an option to prevent code duplication by leveraging TS to create the JS version for the skeleton files. With the exception of files containing the @if/else template syntax I think this could be done. This would at least result in less code and helps improve quality for the TS files and keeps the JS & TS setups identical.

In reply to testing. You mention the templating engine is covered, this is great but does not guarantee the actual templates can not contain mistakes. If you think jest snapshot testing could be useful for this we could run it as a separate task without having to convert the current jasmine setup although it might be relatively easy to convert.

3cp commented 5 years ago

That sounds great!