feat: add Typescript v[4.9.5 ; 5.1.6] support #14
Conversation
| export function hasModifier(node: TS.Node | TS.Modifier[], modifier: TS.Modifier["kind"]): boolean { | ||
| const modifiers = Array.isArray(node) ? node : (node.modifiers as readonly TS.Modifier[] | undefined); | ||
| return modifiers != null && modifiers.some(m => m.kind === modifier); | ||
| export function hasModifier(node: TS.Node | TS.Modifier[], modifier: TS.Modifier["kind"], typescript: typeof TS): boolean { |
There was a problem hiding this comment.
I would prefer a much simpler implementation where we just check for the existence of an array of modifiers on the Node to avoid having to pass in a reference to the TypeScript object and using the new canHaveModifiers helper.
The existing implementation could simply be rewritten to this, which will work in TS5 as well:
/**
* Returns true if the given Node has the given kind of Modifier
*/
export function hasModifier(node: TS.Node | TS.ModifierLike[], modifier: TS.Modifier["kind"]): boolean {
const modifiers = Array.isArray(node) ? node : "modifiers" in node && Array.isArray(node.modifiers) ? (node.modifiers as readonly TS.ModifierLike[]) : undefined;
return modifiers != null && modifiers.some(m => m.kind === modifier);
}| super({ | ||
| message, | ||
| environment, | ||
| node: typescript.factory?.createEmptyStatement() ?? (typescript as any as (typeof TS)["factory"]).createEmptyStatement() |
There was a problem hiding this comment.
This is definitely me being nitpicky, but I'd prefer casting to unknown over any, and maybe a cleaner cast would be: typescript.factory?.createEmptyStatement() ?? (typescript as unknown as TS.NodeFactory).createEmptyStatement() instead :)
| @@ -21,8 +21,8 @@ export function getDotPathFromNode<T extends TS.Node>(options: EvaluatorOptions< | |||
| return SUPER_SYMBOL; | |||
| } else if (typescript.isParenthesizedExpression(node)) { | |||
| return getDotPathFromNode({...options, node: node.expression}); | |||
| } else if (typescript.isTypeAssertionExpression?.(node) || (!("isTypeAssertionExpression" in typescript) && (typescript as typeof TS).isTypeAssertion(node))) { | |||
| return getDotPathFromNode({...options, node: node.expression}); | |||
| } else if (typescript.isTypeAssertionExpression?.(node) || (!("isTypeAssertionExpression" in typescript) && (typescript as any).isTypeAssertion(node))) { | |||
There was a problem hiding this comment.
If instead of the any cast here, we write it like this...:
// ...
else if (
rest.typescript.isTypeAssertionExpression?.(node) ||
(!("isTypeAssertionExpression" in rest.typescript) && (rest.typescript as {isTypeAssertion: typeof TS.isTypeAssertionExpression}).isTypeAssertion(node))
)...
| } else if (typescript.isTypeAssertionExpression?.(node) || (!("isTypeAssertionExpression" in typescript) && (typescript as typeof TS).isTypeAssertion(node))) { | ||
| return getDotPathFromNode({...options, node: node.expression}); | ||
| } else if (typescript.isTypeAssertionExpression?.(node) || (!("isTypeAssertionExpression" in typescript) && (typescript as any).isTypeAssertion(node))) { | ||
| return getDotPathFromNode({...options, node: (node as any as TS.TypeAssertion).expression}); |
There was a problem hiding this comment.
...Then we won't have to do the cast here and can't just keep it as node.expression, as TypeScript will understand that node is a TypeAssertionExpression.
| return expressionContainsSuperKeyword(expression.expression, typescript); | ||
| } | ||
| else { | ||
| else if (typescript.isTypeAssertionExpression?.(expression) || (!("isTypeAssertionExpression" in typescript) && (typescript as any).isTypeAssertion(expression))) { |
There was a problem hiding this comment.
Same feedback as above - we can write this like this instead:
else if (typescript.isTypeAssertionExpression?.(expression) || (!("isTypeAssertionExpression" in typescript) && (typescript as {isTypeAssertion: typeof TS.isTypeAssertionExpression}).isTypeAssertion(expression))) {
return expressionContainsSuperKeyword(expression.expression, typescript);
}That way we won't have to do the expression as TS.TypeAssertion cast
| } | ||
| else { | ||
| else if (typescript.isTypeAssertionExpression?.(expression) || (!("isTypeAssertionExpression" in typescript) && (typescript as any).isTypeAssertion(expression))) { | ||
| return expressionContainsSuperKeyword((expression as TS.TypeAssertion).expression, typescript); |
| return getInnerNode(node.expression, typescript); | ||
| } | ||
| else { | ||
| else if (typescript.isTypeAssertionExpression?.(node) || (!("isTypeAssertionExpression" in typescript) && (typescript as any).isTypeAssertion(node))) { |
There was a problem hiding this comment.
Same feedback as the other two places where we check for a TypeAssertionExpression.
|
thanks for the valuable feedback, this is indeed way better now ! |
|
They're running just fine here on GH Actions. Did you perform an additional |
|
oh oh, couldnt find them |
related to: chakra-ui/panda#1008 (comment)
I got them all !
6860 tests passedHad to use some
anys cause TS deprecated some methods but the update was mostly a painless process !well played on the ts macro versions testing, this helped a lot.
please tell me if there's anything I can do to get this merged