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.

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

Introduce forward references in pretty printer #6069

Merged
merged 1 commit into from
Apr 25, 2018
Merged

Introduce forward references in pretty printer #6069

merged 1 commit into from
Apr 25, 2018

Conversation

mjesun
Copy link
Contributor

@mjesun mjesun commented Apr 25, 2018

Importing an internal patch from @camspiers to make pretty-format aware of forwarded references inside React.

@camspiers
Copy link

Thanks for the quick response @mjesun

For additional context, without this diff snapshot tests for components that use React.forwardRef would end up having a component name of "UNDEFINED", much like the following issues and PR:

#5816
#5786

@codecov-io
Copy link

Codecov Report

Merging #6069 into master will decrease coverage by <.01%.
The diff coverage is 50%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #6069      +/-   ##
==========================================
- Coverage   64.13%   64.12%   -0.01%     
==========================================
  Files         217      217              
  Lines        8328     8332       +4     
  Branches        4        3       -1     
==========================================
+ Hits         5341     5343       +2     
- Misses       2986     2988       +2     
  Partials        1        1
Impacted Files Coverage Δ
...ackages/pretty-format/src/plugins/react_element.js 92% <50%> (-8%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update eb3ba0f...4490282. Read the comment docs.

@mjesun mjesun merged commit ea5f5e0 into jestjs:master Apr 25, 2018
@mjesun mjesun deleted the react-patch branch April 25, 2018 17:24
@camspiers
Copy link

@mjesun Looks like this didn't fix the issue. It needs to be:

  if (typeof element.type === 'object' && element.type !== null
    && element.type.$$typeof === forwardRefSymbol) {
    const functionName =
      element.type.render.displayName || element.type.render.name || '';

    return functionName !== ''
      ? 'ForwardRef(' + functionName + ')'
      : 'ForwardRef';
  }

camspiers added a commit to camspiers/jest that referenced this pull request Apr 30, 2018
Previous PR jestjs#6069 didn't actually fix the issue, as it wasn't correctly comparing the forwardRefSymbol with `element.type.$$typeof`
@github-actions
Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 12, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants