Augment templates configs with more opinionated stricter rules for actor development
Opened this issue · 3 comments
this: https://maximorlov.com/linting-rules-for-asynchronous-code-in-javascript
and also:
- Add rule for
Apify.getValue('INPUT')
->Apify.getInput()
async
event listeners onpage.on()
or any other event listener that is synchronous. if it's there, ask to add try/catch block- for
type: "module"
, require adding the extension for.js
files (not.mjs
) - add
jsconfig.json
by default, a lot of VSCode devs have no idea this exists (and they code contains 0 warnings/errors/hints) and helps a lot with "closer to tsconfig" intellisense and even a companion to eslint
this means it shouldn't be added to top level @apify/eslint-config
because there will be many rules that only make sense while dealing with the SDK on a daily basis
We discussed many times that we should have a @apify/actor-eslint-config
or something similar that will inherit from @apify/eslint-config
.
I think from SDK v3, we plan to fully migrate to ES modules (which we should probably do a long while ago).
Not sure about jsconfig.json
, we would definitely need to disable the all the implicit any
warnings. Better go full Typescript in that case and we don't want to force all users to necessarily do that.
I read the article and I think there isn't much useful rules for writing actors, very rarely you would need to construct promises by hand via new Promise
. And some rules we explicitly want to disable like no-return-await
for stack traces, no-await-in-loop
for a clean and predictable serial execution.
dealing with events (and listeners), dealing with callbacks, this is very common to have new Promise(async (resolve, reject) => {});
and try to async
/await
there there it should be an async IIFE:
let c = 0;
return Promise.race([
new Promise(async (resolve) => {
while (c < 10) {
await page.doSomeScrolling();
c++;
await el = page.$('#id');
if (el) {
resolve(el);
break;
}
}
}),
sleep(30000),
]);
can be solved by IIFE (that it's a promise without the callbacks)
let c = 0;
return Promise.race([
(async () => {
while (c < 10) {
await page.doSomeScrolling();
c++;
await el = page.$('#id');
if (el) {
resolve(el);
break;
}
}
})(),
sleep(30000),
]);
for page events:
const f = (page) => {
return new Promise((resolve, reject) => {
page.on('popup', (newPage) => {
newPage.waitForNavigation().then(() => resolve(newPage), reject);
});
})
}
also I've seen this quite a lot:
await Promises.all(requests.map((req) => requestQueue.addRequest(req)));
// or the good intent one, they are the same
requests.forEach(async (req) => {
await requestQueue.addRequest(req);
});
I've also seen 4, 5, 7, 10 (people don't know there are fs.promises.readFile
). the Typescript ones are almost impossible to make work on plain JS, so that can be left for TS only templates.
and unless we change the ergonomics in SDK 3, we will keep seeing this, so we need better rules.