Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed wrong error being reported on required initialized parameters with isolatedDeclarations #60980

Conversation

Andarist
Copy link
Contributor

@typescript-bot typescript-bot added the For Backlog Bug PRs that fix a backlog bug label Jan 16, 2025
@@ -767,7 +767,7 @@ export function createGetIsolatedDeclarationErrors(resolver: EmitResolver): (nod
if (isSetAccessor(node.parent)) {
return createAccessorTypeError(node.parent);
}
const addUndefined = resolver.requiresAddingImplicitUndefined(node, /*enclosingDeclaration*/ undefined);
const addUndefined = resolver.requiresAddingImplicitUndefined(node, findAncestor(node, isFunctionLikeDeclaration));
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is a somewhat cheap way of fixing this issue, given the importance of the enclosingDeclaration for the correct result resolver.requiresAddingImplicitUndefined, I really wonder if it shouldn't be made a required parameter

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's already required, it's just that it can be undefined, you just don't really want to pass in undefined unless via some goofy callback, the checker gave it undefined.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But, maybe external users should be required to provide it.

@@ -767,7 +767,7 @@ export function createGetIsolatedDeclarationErrors(resolver: EmitResolver): (nod
if (isSetAccessor(node.parent)) {
return createAccessorTypeError(node.parent);
}
const addUndefined = resolver.requiresAddingImplicitUndefined(node, /*enclosingDeclaration*/ undefined);
const addUndefined = resolver.requiresAddingImplicitUndefined(node, node.parent);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was initially worried about this just because enclosingDeclaration is weird and should/shouldn't be certain kinds of nodes, but in this case, node.parent is a function declaration (or expression), so is a valid choice.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(also, no other caller than this ever passes undefined into requiresAddingImplicitUndefined explicitly, so, that's of note)

@jakebailey jakebailey merged commit 0745e6a into microsoft:main Jan 18, 2025
32 checks passed
@Andarist Andarist deleted the fix/isolated-decl-error-required-initializer-param branch January 18, 2025 07:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
For Backlog Bug PRs that fix a backlog bug
Projects
Status: Done
3 participants