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.

Sign up for GitHub

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

Jump to bottom

In addRange, don't check for undefined values; do support slice #16161

Closed
wants to merge 1 commit into from
Closed

In addRange, don't check for undefined values; do support slice #16161

wants to merge 1 commit into from

Conversation

ghost
Copy link

@ghost ghost commented May 31, 2017

@ghost ghost requested a review from rbuckton May 31, 2017 00:28
@rbuckton
Copy link
Member

@Andy-MS, see https://github.com/Microsoft/TypeScript/pull/16120/files#diff-66cec844e251a1918deb897eabca206bR761. Plus, I would prefer we leave the undefined check in.

@ghost
Copy link
Author

ghost commented Jun 2, 2017

Could we use a Debug.assert instead of if for the undefined check?

@rbuckton
Copy link
Member

rbuckton commented Jun 6, 2017

I'd prefer not. Some of the transforms return undefined to indicate that the node should be elided. Since this is used mostly in the transforms, I'd rather be consistent.

@ghost ghost closed this Jun 6, 2017
@ghost ghost deleted the addRange branch June 6, 2017 20:31
@microsoft microsoft locked and limited conversation to collaborators Jun 14, 2018
This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@rbuckton rbuckton Awaiting requested review from rbuckton

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@rbuckton @msftclas