es2017: bump eslint version
This spec was released 7 years ago, and allows e.g. async/await, which can result in more readable code in some cases.
There is danger that we return to the main loop too easily using this construct, so no suggestion to rewrite any of the existing code to use this, as that can have side effects on how e.g. we process incoming websocket messages.
Do use await at a single place to make sure await can be indeed used.
Thanks Hubert to notice that the await-using function itself has to be async, the error message from eslint isn't too clear in that case.
Glad to be using async/await! May we also add no-floating-promises (and possibly no-misused-promises) to eslint as a way to deal with people inevitably forgetting to await where they use this? JavaScript doesn't require awaiting so it's very possible to end up with nasty race conditions etc. if you're not careful and this rule should prevent that in typescript files
Aha. Can you push a new commit to this branch to do this, please? The suggestion sounds sensible to me, but it would be really good to drive this to completion till Thur (next tech planning call) and I fear I won't have time.
Alternatively, I guess we could extend from recommended-type-checked rather than just recommended and it would enable this rule too but that would probably be better in another PR
Yes, probably best to do that separately.
Thanks. :-)
I would like @vmiklos or someone else to look it over as I was the last one to push changes
Sure. Though looks like the main 'Unit & Cypress & Notebookbar Tests against Core co-23.05' job now fails with your changes?
I would like @vmiklos or someone else to look it over as I was the last one to push changes
Sure. Though looks like the main 'Unit & Cypress & Notebookbar Tests against Core co-23.05' job now fails with your changes?
yep, at a glance it looks like it's getting upset at our tsconfig not including all the typescript... I'll take a look
otherwise I'm currently working on the recommended-type-checked PR I suggested earlier... it turns out some of the things we do don't conform very well to it - however by looking at its warnings I've found at least a few places that I am confident could never work (e.g. typeof x === undefined) so I'm rather happy to have it. It'll take a little bit and require me to switch off some of the warnings that are a lost cause (things where the errors from them are in the high 100s) but I think it'll be worth it
Huh, this PR is getting big, it now does 3 changes:
-
bump JS baseline
-
add new eslint rules
-
apart from just a single sample use of await, it adds multiple uses, refactors, etc.
Seeing that CI is still red, should we split this to 2 pieces, keep just 1) here and have a next PR for 2) & 3)? What would you prefer, @Minion3665 ?
Huh, this PR is getting big, it now does 3 changes:
1. bump JS baseline 2. add new eslint rules 3. apart from just a single sample use of await, it adds multiple uses, refactors, etc.Seeing that CI is still red, should we split this to 2 pieces, keep just 1) here and have a next PR for 2) & 3)? What would you prefer, @Minion3665 ?
yes, alright, that makes sense. I guess I might want to roll 2 into the recommended-type-checked thing I'm working on... Not sure where 3 goes but maybe a new PR
https://cpci.cbg.collabora.co.uk:8080/job/github_online_master_debug_vs_co-23.05/10313/console
writer/track_changes_spec.js failed, retry.
https://cpci.cbg.collabora.co.uk:8080/job/github_online_master_debug_vs_co-23.05/10324/console
writer/invalidations_spec.js and writer/track_changes_spec.js failed, retry.
@eszkadev could you please review this? Thanks.
Bump the baseline to ES2017 + introduce minimal usage of async/await.