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

displayName is added to regular JS objects #19

Open
iMoses-Apiiro opened this issue Jan 13, 2022 · 4 comments
Open

displayName is added to regular JS objects #19

iMoses-Apiiro opened this issue Jan 13, 2022 · 4 comments

Comments

@iMoses-Apiiro
Copy link

iMoses-Apiiro commented Jan 13, 2022

I have a large map in my code that looks something like this:

export const MANAGE_ACTIONS = {
  actionType: {
    nameFunc: () => {},
    iconFunc: () => {},
  },
 ...
};

which I elsewhere use to fetch the relevant functions per actionType so I'm expecting the values to all be objects,
and now all of a sudden I'm getting:

MANAGE_ACTIONS.displayName === 'MANAGE_ACTIONS' // true

After a short debugging this seems to be the problem:

const parentAncestor = ancestors[ancestors.length - 2]

if (parentAncestor && ['ReturnStatement', 'ArrowFunctionExpression'].includes(parentAncestor.type)) {
  // ArrowFunctionExpression is present when no Babel plugins are used when transforming JSX

  const variableDeclaratorIdx = ancestors.findIndex(ancestor => ancestor.type === 'VariableDeclarator')

  if (variableDeclaratorIdx != -1) {
    const variableDeclarator = ancestors[variableDeclaratorIdx]
    addDisplayName(parser, variableDeclarator)
  }
}

the assumption here about arrow functions is a little too generic :)

Expected
if nameFunc returns JSX then MANAGE_ACTIONS.actionType.nameFunc.displayName === 'nameFunc'

Actual
MANAGE_ACTIONS.displayName === 'MANAGE_ACTIONS'

@tmcneal
Copy link
Contributor

tmcneal commented Jan 18, 2022

Thanks for the report! This is indeed a bug, I am able to reproduce it with the following code:

import { HashRouter } from "react-router-dom"

const JSXExample = <HashRouter></HashRouter>

export const MANAGE_ACTIONS = {
  actionType: {
    nameFunc: () => { return JSXExample },
    iconFunc: () => {},
  },
};

@iMoses
Copy link

iMoses commented Jan 19, 2022

I'd be happy to assist however I can, it seems that the walking logic and addDisplayName needs to be adjusted differently to handle these situations. was thinking maybe ansectors should be passed down the name naming method along with the root node of the matched function and the building of the path shuold be made there accroding to consistent logic.
wdyt?

@tmcneal
Copy link
Contributor

tmcneal commented Jan 20, 2022

I think you're right thatsomething with handling of ancestors will definitely need to change since that's where it's finding 'JSXExample' in the example code above. I took a look, and the logic that's checking the ancestors is used to detect references like this React.memo call: https://github.com/runreflect/webpack-react-component-name/blob/master/examples/memo/src/MemoizedButton.js#L4

If you comment out the addDisplayName call on line 113 of index.js, you'll see that the following unit test fails: https://github.com/runreflect/webpack-react-component-name/blob/master/test/plugin.spec.js#L178-L193. In this scenario, the plugin is converting this code:

const MemoizedButton = React.memo(Button);

to this when it's minified:

var J=r.memo(X);try{J.displayName="MemoizedButton"}catch(e){}

So I think what we need is a way to preserve that behavior without having false positive matches like the one you reported. If you're willing to assist, I'll definitely review and merge the PR.

Thanks again for reporting!

@tmcneal
Copy link
Contributor

tmcneal commented Feb 2, 2022

Hi @iMoses-Apiiro I will start work on a fix here soon but if you're planning on opening a PR just let me know and I'll hold off.

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

3 participants