Skip to content

[bug] act-compat assume console.error is called with a string #442

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.

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

Closed
SBoudrias opened this issue Aug 14, 2019 · 7 comments
Closed

[bug] act-compat assume console.error is called with a string #442

SBoudrias opened this issue Aug 14, 2019 · 7 comments

Comments

@SBoudrias
Copy link

  • react-testing-library version:
  • react version:
  • node version:
  • npm (or yarn) version:

Relevant code or config:

Code is situated here: https://github.com/testing-library/react-testing-library/blob/master/src/act-compat.js#L32

args[0].indexOf(
              'Warning: Do not await the result of calling ReactTestUtils.act',
            ) === 0

This assumes the first arg passed to console.error is a string; but the type could be different. For example, it can be an object console.error({ error })

Suggested solution:

We could either:

  1. Stringify the args before calling indexOf
  2. Type check the arg first
@alexkrolick
Copy link
Collaborator

alexkrolick commented Aug 14, 2019

Type check the arg first

This seems fine, since the condition is always false (never matches) when the input is not a string

@kentcdodds
Copy link
Member

Agreed. Would you like to make a pull request for this @SBoudrias?

@ely-alamillo
Copy link

ely-alamillo commented Aug 20, 2019

@kentcdodds I can go ahead and add some type checking to the args for this.

@rvdkooy
Copy link
Contributor

rvdkooy commented Aug 26, 2019

@ely-alamillo you still taking this? I can also do this one?

@ely-alamillo
Copy link

@rvdkooy yes, there was a blocking issue but it's now resolved so i'm still working on this.

If I don't get anything in by the eow you can take it.

@ely-alamillo
Copy link

@kentcdodds
Copy link
Member

Because otherwise this would run:

originalConsoleError.apply(console, args)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants