Convert Aurelia CLI `/lib` code to TypeScript
Hi,
I converted JavaScript files from /lib folder to TypeScript. More percisely:
- TypeScript source code is on
/srcfolder andoutDiris/lib. - ESLint configuration has been updated to support TypeScript.
- TypeScript compilation (
npm run buildornpx tsc) overwrites existing files*.jsfiles in/libfolder. - Generated
*.jsfiles are still CommonJS modules, backward compatible with previous version. - There was no change
/bin/aurelia-cli.js. - There was no change Jasmine unit-tests. All tests are passing.
- ~~Added example to
ReadMe.mdhow to run single Jasmine unit-test (e.g.npx jasmine spec/lib/build/bundled-source.spec.mjs --filter="transform saves cache")~~
My motivation was, since company where I work still has many Aurelia v1 projects, to make the code easier to understand and, perhaps, lower barrier for future contributions.
I tested:
- Unit tests
- Compilation of larger TypeScript based Aurelia web that uses various npm packages, including one
wrapShipforleaflet.drawmodule. I tested the web itself in the browser succesfully. - Tested
au generate component my-component. - Created Babel based Aurelia project, with sample app code (input binding, Github page, child router) compiled it and ran it succesfully in browser.
This pull request is 3rd attempt upgrade:
- First attempt used
*.mtsmodules, but I ran into issue with mocking ofsrc/build/utils.mtsmodule in Jasmine unit tests. Methods on ESM modules cannot be overwritten, like methods on CommonJS modules. There are multiple approaches for mocking of ESM modules in NodeJS, but it seems they have been changing in every version (e.g.--loader,--experimetal-loader, now recommndation is to use--importand*.jsfile that usesmodule.register()to register custom loader hooks). - CommonJS attempt with all
requirecommand changed withimportorawait import. Eventually I rolled back torequiresince I was troubleshooting issue (Cannot find module '@babel/plugin-transform-modules-amd') that ended up being in the filesrc/build/amodro-trace/read/es.ts. Loading of Babel plugin'@babel/plugin-transform-modules-amd'viapluginsoption oftransform(...)method did not work, so plugin is loaded in the header of the file withregistercommand now. I suspect the culprit is that I used symbolic link foraurelia-cli(npm link aurelia-cli). And I switched fromtransformtotransformSync, since former is going to be removed in Babel v8. This was only notable issue I had. - Third iteration is this pull request. Code is identical to 2nd attempt, I just reduced number of Git commits to 5.
Going forward from this version it would be possible to further improve type checking, update more of the old JS syntax, use asynchrounos module loading await import where feasable and so on.
Let me know if this is something that would be interesting for you to merge in Aurelia CLI repository and if you would like me to make further changes.
Update:
- Fixed error:
Error:minificationResult.mapshould be string!when minifying production bundle. - Added example to
ReadMe.mdhow to run single Jasmine unit-test (e.g.npx jasmine spec/lib/build/bundled-source.spec.mjs --filter="transform saves cache")
Best, Nenad
Thanks for the effort, @nenadvicentic! However, we do wish that you discussed the idea here before spending so much time.
au1 cli is in maintenance mode. I am not sure do we want to do the ts convertion @bigopon? My concern is that our unit test coverage is not that great, so it has risk of missing some regression.
About the change itself, since the lib/ files are dist files, it can be called dist/. And we don't need the whole lib/ folder committed in git at all, they are output of tsc. That build task can be took care of by setting up "prepare" or "prepublish" npm script.
Hi @3cp,
No problem for the effort. Regarding having no prior discussion - I started this in an attempt to understand what is going under the hood of the CLI. And I kept going. It's is completely OK if you think it's better to put this on the shelf.
I did my best to make only usefull and low-risk changes, in order to minimise chance of errors/regressions. I agree with you about the test coverage. What you have covered is helpful, but there are many things that are not covered by unit tests. The best test for this version is to use this version on real-life projects. I can do it with 4-5 projects, but admitedly, they all follow similar pattern (CLI + TypeScript + SASS/LESS).
In case you guys decide to proceed, I can make suggested change to /dist, but probably I would need more input from you, if I use some build tool or just write copy-script for *.json and logo.text files. The Bluebird configuration files in lib/resources/scripts/ should stay there, because they are referenced from aurelia.json in older Aurelia apps.
@3cp Btw, I noticed that in package.json there is invalid entry: "gulp": ">=4.0.2".
This does not cause an issue in Aurelia apps, because they have correct entry "gulp": "^4.0.2". However, standalone aurelia-cli repository picks the latest version, which is right now 5.0.0.
The cli and v1 skeleton has not been touched for years. We have to lock gulp to v4, as v5 has many unfixed issues.
I have updated cli package.json. The v1 skeleton already has gulp:^4.0.2.
au1 cli is in maintenance mode. I am not sure do we want to do the ts convertion @bigopon? My concern is that our unit test coverage is not that great, so it has risk of missing some regression.
I dont think we shoul make change to a in-maintenance repo, except when someone really wishes to do so and willing to prove their determination via their work. If this condition happens, then I think its good to have. But you make the call @3cp .
I think we hold this one for now.
Cli is only must have in a cli-bundler based app.
For webpack based app (skeleton still use cli as task runner), or dumber based app, cli is irrelevant or can be removed.
@3cp, @bigopon thank you both for taking the time to look into this.
I agree with you, it's a pretty big change, let's put the merge on hold and have more time to think about it. I would like just to add some more details.
This feature branch
I would still like to tidy-up this branch (dist folder, add more simple types) in coming days, even if only for myself. I would also love you to share some more feedback with me, if you have time.
Finally, I would encourage you to try it in your own cli based projects:
npm install -d "https://github.com/nenadvicentic/cli.git#feature/typescript"
From everything I tested to far, this really works.
Aurelia CLI in general
Cli is only must have in a cli-bundler based app. For webpack based app (skeleton still use cli as task runner), or dumber based app, cli is irrelevant or can be removed.
I understand your point of view, specially in light of Aurelia v2, but from a perspective of a company that has Aurelia v1 webs in production, aurelia-cli projects are not going to go away any time soon. aurelia-cli was also the most stable from the three options over long periods of time.
I am sure that you guys agree with me, at least partially, because there is an effort to refresh v1 documentation and also recently did #1210 , based on issue I filed: Update package.json dependencies for new projects. So it should not be left to die.
All that said - thank you both again for your time and effort you put in Aurelia and in looking into this pull request.
Hi @3cp, @bigopon,
I wanted to update you on the status of this pull request.
I've been using this final version of the pull request in multiple Aurelia CLI production projects for about three weeks, and it has proven to be stable, with no further modifications needed.
Bellow is the summary of where we ended up.
Commit Structure
The PR is now cleanly rebased into two logical commits:
- Migration of
*.jsfiles from/libto/srcwith TypeScript conversion - All subsequent improvements in a single follow-up commit
Change Breakdown
Changes fall into four clear categories:
- Type annotations (∼90% of changes)
- Async/await conversions where they improved clarity (2-3%)
- Readability improvements from ESLint/TypeScript rules
- Unit test updates to reference
/distinstead of/src
Key Improvements
- I implemented @3cp's suggestion for
/distoutput (excluded from Git) using npmprepare. - Since TypeScript now also has to run upon NPM package installation, to generate
/distfolder content, I updated minimum Node.js to 14.17 (required for TS 5.1+) - Since this version of Node.js has newer file-system methods (e.g.
node:fs/promises, I simplified and cleaned upsrc/file-system.ts. This is the only file in the repository that has significant changes (reduction of lines).
Testing Validation
The changes have been tested across:
Production Environments
- Multiple Aurelia CLI projects with combinations including:
- TypeScript + SCSS + Bootstrap 5
- TypeScript + Less + Bluebird + Bootstrap 3
CLI Functionality
- All
au generatecommands validated (both JS and TS)
New Project Templates
-
TypeScript (CLI Bundler)
npx makes aurelia/v1 -s cli-bundler,typescript,htmlmin,less,postcss,jest,playwright,vscode,scaffold-navigation- Verified: build, start, test, and e2e workflows
-
Babel (CLI Bundler)
npx makes aurelia/v1 -s cli-bundler,alameda,htmlmin,sass,postcss,jest,playwright,vscode,scaffold-navigation- Verified: build, start, test, and e2e workflows
-
TypeScript (Webpack)
npx makes aurelia/v1 -s typescript,htmlmin,sass,postcss,jest,playwright,vscode,scaffold-navigation- Note:
src/build/webpack-reporter.tsappears unused in current implementation
- Note:
Final Outcome
- 100% backward compatible with 3.0.4 (except Node ≥14.17 requirement)
- Produces bit-identical output to 3.0.4
- Delivers full type support for enhanced IDE experience
- Significantly improves maintainability via TypeScript
Again, I completely understand if you prefer to keep this pull request on hold, but please, when you have some time, take a look into it. In the meantime I won't be making any further changes on my own. If you have any thoughts, questions or suggestions, do let me know.