chore: upgrade typescript and eslint#149
Conversation
emmclaughlin
left a comment
emmclaughlin
left a comment
There was a problem hiding this comment.
Thank you for the PR! Just one comment. Is is possible to avoid the two ts-expect-error?
| // @ts-expect-error - no types available for mjml-preset-core | ||
| import * as presetCore from "mjml-preset-core"; |
There was a problem hiding this comment.
@ts-expect-error will error the day mjml-preset-core ships types, since the expected error won't be there anymore. A small ambient types/mjml-preset-core.d.ts like
declare module "mjml-preset-core" {
export const components: unknown[];
}would let you drop both @ts-expect-error lines (this file and the test) and is more durable.
There was a problem hiding this comment.
I can check if declaration file helps. Although I think it is better to error when the types ship, as some code can be deleted then. Deleting code - always better than writing code.
There was a problem hiding this comment.
done. I added the declaration file.
There was a problem hiding this comment.
That's fair, I'm just programmed to not like an override to skip an error. Thanks for the update
| import * as fs from "fs"; | ||
| import camelCase from "lodash.camelcase"; | ||
| import upperFirst from "lodash.upperfirst"; | ||
| import * as presetCore from "mjml-preset-core"; |
There was a problem hiding this comment.
Weird, the test passed even though it failed at this step. Will help take a quick look
There was a problem hiding this comment.
@anajavi sorry for the churn, but I think it makes sense to revert back to your original implementation and use @ts-expect-error in both files and delete scripts/types/mjml-preset-core.d.ts
There was a problem hiding this comment.
no worries. I reverted the change.
This reverts commit 20dfbec.
|
Looks good! Thanks for upgrading |
Thanks for the fast review. Happy to help. |
2 participants
Upgrades typescript and typescript-eslint to newer versions.
I had to change a couple of require() calls to imports as newer typescript-eslint started giving errors about rule @typescript-eslint/no-require-imports.
I did not update to typescript 6 as that one requires a bit more thought.