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

[4.x] fix(core): stringify shouldn't throw when toString returns null/undefined #14975

Closed
wants to merge 1 commit into from
Closed

[4.x] fix(core): stringify shouldn't throw when toString returns null/undefined #14975

wants to merge 1 commit into from

Conversation

DzmitryShylovich
Copy link
Contributor

Fixes #14948

@DzmitryShylovich DzmitryShylovich force-pushed the gh/4.x/14948 branch 2 times, most recently from 11f1086 to af553d7 Compare March 12, 2017 15:47
vicb
vicb reviewed Mar 21, 2017
View reviewed changes
packages/core/test/util_spec.ts Outdated
class Foo {
toString(): string { return undefined; }
}
expect(stringify(new Foo())).toBe('undefined');
Copy link
Contributor

Choose a reason for hiding this comment

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

expect(stringify({toString => undefined})).toBe('undefined');

vicb
vicb approved these changes Mar 21, 2017
View reviewed changes
Copy link
Contributor

@vicb vicb left a comment

Choose a reason for hiding this comment

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

nit: could simplify the test

@vicb vicb added area: core Issues related to the framework runtime action: merge The PR is ready for merge by the caretaker labels Mar 21, 2017
@DzmitryShylovich
Copy link
Contributor Author

DzmitryShylovich commented Mar 21, 2017 •
edited
Loading

@vicb done. simplified the tests + rebase

vicb
vicb reviewed Mar 21, 2017
View reviewed changes
packages/core/test/util_spec.ts Outdated
export function main() {
describe('stringify', () => {
it('should return string undefined when toString returns undefined',
() => expect(stringify({toString: (): string => undefined})).toBe('undefined'));
Copy link
Contributor

Choose a reason for hiding this comment

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

string -> any

@vicb
Copy link
Contributor

vicb commented Mar 21, 2017

@DzmitryShylovich thanks ! Could you please update the return type

@DzmitryShylovich
Copy link
Contributor Author

@vicb done

@mhevery mhevery closed this in 8e6995c Mar 21, 2017
@DzmitryShylovich DzmitryShylovich deleted the gh/4.x/14948 branch March 21, 2017 19:33
asnowwolf pushed a commit to asnowwolf/angular that referenced this pull request Aug 11, 2017
@DzmitryShylovich
fix(core): stringify shouldn't throw when toString returns null/undef…
9b95fb6 
…ined (angular#14975)

Fixes angular#14948

PR Close angular#14975
juleskremer pushed a commit to juleskremer/angular that referenced this pull request Aug 28, 2017
@DzmitryShylovich @juleskremer
fix(core): stringify shouldn't throw when toString returns null/undef…
e54e865 
…ined (angular#14975)

Fixes angular#14948

PR Close angular#14975
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 11, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@vicb vicb vicb approved these changes

Assignees
No one assigned
Labels
action: merge The PR is ready for merge by the caretaker area: core Issues related to the framework runtime cla: yes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

stringify fails if token.toString() returns undefined
4 participants
@DzmitryShylovich @vicb @mhevery @googlebot