Skip to content

use modulekind to check if initializer for shorthand property assignm…#5576

Merged
vladima merged 2 commits into
masterfrom
importsInShorthandProps
Nov 9, 2015
Merged

use modulekind to check if initializer for shorthand property assignm…#5576
vladima merged 2 commits into
masterfrom
importsInShorthandProps

Conversation

@vladima
Copy link
Copy Markdown
Contributor

@vladima vladima commented Nov 9, 2015

…ent should be emitted. Fixes #5573

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.

Are these three errors part of the test? From reading the bug, it looks like only foo1 and const test = { x }; are needed and the rest can be removed.

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.

console is irrelevant, However missing ./foo2 module is necessary to make sure that we emit the right thing no matter whether module is found or not.

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.

Can you name the modules found.ts/notFound.ts then?

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.

sure

@sandersn
Copy link
Copy Markdown
Member

sandersn commented Nov 9, 2015

Looks good except for maybe some test cleanup.

@mhegazy
Copy link
Copy Markdown
Contributor

mhegazy commented Nov 9, 2015

👍

vladima added a commit that referenced this pull request Nov 9, 2015
use modulekind to check if initializer for shorthand property assignm…
@vladima vladima merged commit 5d9e7e1 into master Nov 9, 2015
@vladima vladima deleted the importsInShorthandProps branch November 9, 2015 22:57
@microsoft microsoft locked and limited conversation to collaborators Jun 19, 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.

4 participants