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

Issue an error on cross-file merges we can't emit #38148

Merged

Conversation

weswigham
Copy link
Member

Fixes #34994

I asked @sandersn and he was more comfortable issuing an error (at least for now) than attempting a fix that automatically wrote augmentation declarations.

Copy link
Member

@sandersn sandersn left a comment

Choose a reason for hiding this comment

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

Looks good, although it's a little weird that tracker and reportNonlocalAugmentation might be optional.

const name = unescapeLeadingUnderscores(s.escapedName);
const localName = getInternalSymbolName(s, name);
const aliasDecl = s.declarations && getDeclarationOfAliasSymbol(s);
if (containingFile && (aliasDecl ? containingFile !== getSourceFileOfNode(aliasDecl) : !some(s.declarations, d => getSourceFileOfNode(d) === containingFile))) {
context.tracker?.reportNonlocalAugmentation?.(containingFile, symbol, s);
Copy link
Member

Choose a reason for hiding this comment

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

that is an impressive number of ?. there

@andrewbranch
Copy link
Member

Is this PR relevant to #37896?

@weswigham
Copy link
Member Author

Is this PR relevant to #37896?

Don't think so - this is mostly just possible because we allow module.exports member assignment augmentations in JS, even when it's, as a whole, assigned to something else (and that something else may come from a different file).

@weswigham weswigham merged commit 815dc90 into microsoft:master Apr 24, 2020
@weswigham weswigham deleted the js-declarations-crossfile-merge branch April 24, 2020 02:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Strange declarations created within typescript-in-jsdoc
5 participants