Skip to content

Fix class name emit in ES5#15379

Merged
rbuckton merged 2 commits into
masterfrom
fix14945
Apr 25, 2017
Merged

Fix class name emit in ES5#15379
rbuckton merged 2 commits into
masterfrom
fix14945

Conversation

@rbuckton
Copy link
Copy Markdown
Contributor

This fixes the emit for class names when emitting classes for ES5.

Fixes #14945

Copy link
Copy Markdown
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.

One nit, otherwise looks good.

return false;
}
const blockScope = getEnclosingBlockScopeContainer(declaration);
while (currentNode) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

you can use findAncestor in core.ts to replace this loop. It's fairly new, about a week or so old.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

findAncestor doesn't do much to improve this. It's the same number of lines, introduces a new environment record for the closure each time it's called, and doesn't improve readability of the function. There's no apparent performance difference between the two, as far as I can tell.

        function isPartOfClassBody(declaration: ClassLikeDeclaration, node: Identifier) {
            node = getParseTreeNode(node, isIdentifier);
            if (!node || node.end <= declaration.pos || node.pos >= declaration.end) {
                // if the node has no correlation to a parse tree node, its definitely not
                // part of the body.
                // if the node is outside of the document range of the declaration, its
                // definitely not part of the body.
                return false;
            }
            const blockScope = getEnclosingBlockScopeContainer(declaration);
            return !!findAncestor(node, current => {
                if (current === blockScope || current === declaration) {
                    return "quit";
                }
                if (isClassElement(current) && current.parent === declaration) {
                    // we are in the class body, but we treat static fields as outside of the class body
                    return current.kind !== SyntaxKind.PropertyDeclaration
                        || (getModifierFlags(current) & ModifierFlags.Static) === 0 ? true : "quit";
                }
                return false;
            });
        }

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Fair enough. If there's no readability benefit then there's no point introducing it.

@rbuckton rbuckton merged commit 6756e3e into master Apr 25, 2017
@rbuckton rbuckton deleted the fix14945 branch April 25, 2017 22:09
@microsoft microsoft locked and limited conversation to collaborators Jun 14, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Incorrect emit es5 class

3 participants