Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
michaelw85 opened this issue Jun 15, 2019 · 6 comments
Open

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

michaelw85 opened this issue Jun 15, 2019 · 6 comments

Comments

@michaelw85
Copy link
Contributor

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.

  • IDE does not know what to do with these files (work around: manually setting file type)
  • Linter don't work on these files
  • Not unit testable, files cannot be required as-is
  • These files are never properly setup for TS since the typings would not work in a JS output which leads me to:
  1. 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 #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
Copy link
Member

3cp commented Jun 15, 2019

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
Copy link
Contributor Author

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
Copy link
Member

3cp commented Jun 15, 2019

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
Copy link
Member

3cp commented Jun 15, 2019

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

@michaelw85
Copy link
Contributor Author

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
Copy link
Member

3cp commented Jun 16, 2019

That sounds great!

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

No branches or pull requests

2 participants