Skip to content

Warn about unsafe toWarnDev() nesting in tests#12457

Merged
bvaughn merged 6 commits into
facebook:masterfrom
bvaughn:issues/12428
Aug 21, 2018
Merged

Warn about unsafe toWarnDev() nesting in tests#12457
bvaughn merged 6 commits into
facebook:masterfrom
bvaughn:issues/12428

Conversation

@bvaughn
Copy link
Copy Markdown
Contributor

@bvaughn bvaughn commented Mar 26, 2018

Resolves #12428

This is a work in progress because we have 3 tests (currently marked with ESLint suppression comments) that fail if the nesting order is reversed. (When we replay failed work, the warning is logged a second time- even though we only expect it once.)

@facebook-github-bot
Copy link
Copy Markdown
Contributor

Thank you for your pull request. We require contributors to sign our Contributor License Agreement, and yours has expired.

Before we can review or merge your code, we need you to email cla@fb.com with your details so we can update your status.

@gaearon
Copy link
Copy Markdown
Collaborator

gaearon commented Aug 7, 2018

What's missing?

@facebook-github-bot
Copy link
Copy Markdown
Contributor

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

@gaearon
Copy link
Copy Markdown
Collaborator

gaearon commented Aug 7, 2018

nooooooo

@facebook-github-bot
Copy link
Copy Markdown
Contributor

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

1 similar comment
@facebook-github-bot
Copy link
Copy Markdown
Contributor

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

@bvaughn
Copy link
Copy Markdown
Contributor Author

bvaughn commented Aug 7, 2018

What's missing

Uh, I had totally forgotten about this PR. I guess it's just those three lint rules. I could pick this back up and finish it if we think it's worth doing.

@facebook-github-bot
Copy link
Copy Markdown
Contributor

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

4 similar comments
@facebook-github-bot
Copy link
Copy Markdown
Contributor

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

@facebook-github-bot
Copy link
Copy Markdown
Contributor

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

@facebook-github-bot
Copy link
Copy Markdown
Contributor

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

@facebook-github-bot
Copy link
Copy Markdown
Contributor

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

@aweary
Copy link
Copy Markdown
Contributor

aweary commented Aug 8, 2018

@bvaughn you must have really knocked it out the park signing that CLA

@facebook-github-bot
Copy link
Copy Markdown
Contributor

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

@bvaughn
Copy link
Copy Markdown
Contributor Author

bvaughn commented Aug 8, 2018

I signed the heck out of it

@sebmarkbage
Copy link
Copy Markdown
Contributor

Is this still active? WIP?

@bvaughn
Copy link
Copy Markdown
Contributor Author

bvaughn commented Aug 16, 2018

@facebook-github-bot seems pretty active here

But otherwise, no. I'd forgotten about it.

@gaearon
Copy link
Copy Markdown
Collaborator

gaearon commented Aug 16, 2018

FWIW I did get confused by our existing tests that had wrong nesting a few days ago. I think we should fix.

@bvaughn
Copy link
Copy Markdown
Contributor Author

bvaughn commented Aug 20, 2018

I'll see if I can finish this one out then.

@bvaughn bvaughn requested review from gaearon and sebmarkbage August 20, 2018 17:57
@bvaughn
Copy link
Copy Markdown
Contributor Author

bvaughn commented Aug 20, 2018

Removed the two lint-warning-suppression comments.

@bvaughn bvaughn changed the title [WIP] Warn about unsafe toWarnDev() nesting in tests Warn about unsafe toWarnDev() nesting in tests Aug 20, 2018
'Use the `defaultValue` or `value` props instead of setting children on <textarea>.',
);
}).toThrow();
).toThrow();
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

While we’re at it, can we add a message here so that we don’t pass on any error?

@bvaughn bvaughn merged commit e106b8c into facebook:master Aug 21, 2018
@bvaughn bvaughn deleted the issues/12428 branch August 21, 2018 14:43
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.

6 participants