Skip to content

It must be possible for ES6 defaultProps be equal to function execution #2104

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
fabionalves opened this issue Jan 2, 2019 · 16 comments
Closed
Assignees
Labels

Comments

@fabionalves
Copy link

fabionalves commented Jan 2, 2019

In the class below:

class TestComponent extends React.Component {

    static defaultProps = function () {

       const date = new Date();

        return {
            date
        };
    }();
}

the defaultProps are equal to a anonymous function return value.

In this case, the eslint-plugin-react doesn't run, and throws the following error:
TypeError: Cannot read property 'split' of undefined at Object.isPropWrapperFunction (~/project/node_modules/eslint-plugin-react/lib/util/propWrapper.js:12:26) at resolveNodeValue (~/project/node_modules/eslint-plugin-react/lib/util/defaultProps.js:30:23) at Object.ClassProperty (~/project/node_modules/eslint-plugin-react/lib/util/defaultProps.js:222:26) at updatedRuleInstructions.(anonymous function) (~/project/node_modules/eslint-plugin-react/lib/util/Components.js:752:46) at listeners.(anonymous function).forEach.listener (~/project/node_modules/eslint/lib/util/safe-emitter.js:45:58)

I'm using the following versions:
"eslint": "5.10.0"
"eslint-plugin-react": "7.11.1".

If in the same class I put the defaultProps equal do an object it runs as expected.

Thanks

@ljharb
Copy link
Member

ljharb commented Jan 2, 2019

The rule definitely shouldn't crash - but static defaultProps can only ever be an object literal, not a function. The legacy createReactClass pattern is the one that had a function for things like that.

@ljharb
Copy link
Member

ljharb commented Jan 2, 2019

Can you update to v7.12.1 and see if it's still crashing?

@ljharb
Copy link
Member

ljharb commented Jan 2, 2019

Also, do you know what rule it's crashing on?

@alexzherdev
Copy link
Contributor

@ljharb this is an IIFE though, so it should still work out? Pretty rare case though and not something we can lint for IMO (shouldn't crash of course though).

@ljharb
Copy link
Member

ljharb commented Jan 2, 2019

Ah, I didn't even see that - yes, the IIFE should work fine wrt React.

@jethrolarson
Copy link

I'm also getting this with latest

    "eslint": "5.11.1",
    "eslint-plugin-react": "7.12.1",

Setting this in .eslintrc.yaml is my work-around
react/forbid-prop-types: 0

ST:

> node ./node_modules/eslint/bin/eslint.js --ext .js,.jsx .
TypeError: Cannot read property 'split' of undefined
    at Object.isPropWrapperFunction (/project/node_modules/eslint-plugin-react/lib/util/propWrapper.js:12:26)
    at checkNode (/project/node_modules/eslint-plugin-react/lib/rules/forbid-prop-types.js:128:31)
    at MemberExpression (/project/node_modules/eslint-plugin-react/lib/rules/forbid-prop-types.js:158:9)
    at listeners.(anonymous function).forEach.listener (/project/node_modules/eslint/lib/util/safe-emitter.js:45:58)
    at Array.forEach (<anonymous>)
    at Object.emit (/project/node_modules/eslint/lib/util/safe-emitter.js:45:38)
    at NodeEventGenerator.applySelector (/project/node_modules/eslint/lib/util/node-event-generator.js:251:26)
    at NodeEventGenerator.applySelectors (/project/node_modules/eslint/lib/util/node-event-generator.js:280:22)
    at NodeEventGenerator.enterNode (/project/node_modules/eslint/lib/util/node-event-generator.js:294:14)
    at CodePathAnalyzer.enterNode (/project/node_modules/eslint/lib/code-path-analysis/code-path-analyzer.js:632:23)

@ljharb
Copy link
Member

ljharb commented Jan 2, 2019

@jethrolarson by any chance, can you share the code that it's erroring on?

@jethrolarson
Copy link

Stack trace doesn't say where the actual problem is, so I'm not sure. Code is proprietary so I can't just share the project.

@ljharb
Copy link
Member

ljharb commented Jan 2, 2019

The OP's test code doesn't crash on master.

@jethrolarson I understand. if you edit lib/rules/forbid-prop-types.js line 128 to console.log(context.getFilename()) (i think), it should print out the file at least?

@alexzherdev
Copy link
Contributor

@ljharb just throwing it out there - would it be impossible for eslint to set an uncaught exception handler that would make it easier for people to locate the source of crashes? We've been getting a lot of reports like this recently. (also sorry for having caused some of them with my changes)

@ljharb
Copy link
Member

ljharb commented Jan 2, 2019

I suppose we could; it'd be pretty intrusive to other plugins tho. It seems more like something eslint itself should be handing.

@jethrolarson
Copy link

File with error (i think):

HeroTeaserList.propTypes = Object.assign({
    heroIndex: PropTypes.number,
    preview: PropTypes.bool,
}, componentApi, teaserListProps);

in another file

export const componentConfiguration = {
  childComponentMapping: objectOf(string),
  componentConfig: object,                
  componentInstanceId: string,            
  componentType: string,                  
  dataType: string,                       
  filter: shape(TemplateFilter),          
  filterVersionSetUuid: string,           
  teasers: arrayOf(shape(TeaserContent)), 
  title: string,                          
  titleOverride: node,                    
  uuid: string.isRequired,                
  visibleCount: number,                   
  epgUuid: string,                        
  readOnly: bool,                         
};

export const componentApi = Object.assign({
  getComponent: func.isRequired,
  onNavigation: func,
  onConfigurationChange: func,
}, componentConfiguration);

another file

export const teaserListProps = {
  'componentInstanceId': string,
  'title':               string.isRequired,
  'uuid':                string.isRequired,
  'data':                object,
  'listSize':            number,
  'priority':            number,
  'teasers':             arrayOf(shape(teaserProps)).isRequired,
  'visibleCount':        number,
};

@ljharb
Copy link
Member

ljharb commented Jan 2, 2019

Perfect, I'm able to repro.

@alexzherdev
Copy link
Contributor

@ljharb yeah that's what I meant there; just thought you might know why that could be unrealistic. I'll look around their issue tracker. 👍

@ghost ghost mentioned this issue Jan 12, 2019
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

No branches or pull requests

5 participants