DevExpress / testcafe-react-selectors

TestCafe selector extensions for React apps.
https://testcafe.io
MIT License
205 stars 43 forks source link

TypeError: withProps method error reading 'hasOwnProperty' from props #196

Closed denisputnov closed 1 year ago

denisputnov commented 1 year ago

Hi!

The problem:

I have faced the next problem in the my production code:

TypeError: Cannot read properties of null (reading 'hasOwnProperty')

It happens in code:

ReactSelector('Component')
        .withProps({ visible: true })
        .findReact('Component2')
        .with({ boundTestRun: someTestController });

The error occurs constantly on each test run.

Reproduction:

"testcafe": "2.6.0",
"testcafe-browser-tools": "2.0.23",
"testcafe-react-selectors": "5.0.2",

"react": "18.2.0"

At some point, I went from debugging my own code to debugging a library and noticed that the error occurred there.

I came across an implementation of the componentHasProps (view in code) method and noticed that sometimes null comes as props

function componentHasProps ({ props }, filterProps, exactObjectMatch) {
                               ^---- props is null here

     for (const prop of Object.keys(filterProps)) {
         if (!props.hasOwnProperty(prop)) return false;

         if (!matchProps(props[prop], filterProps[prop], exactObjectMatch))
             return false;
     }

     return true;
}

So I have tried to recreate this error in some playground to attach it to this issue, but it didn't work out for me.

Unfortunately, I can not attach any production code due NDA and can not reproduce it in the playground.

My suggestion:

function componentHasProps ({ props }, filterProps, exactObjectMatch) {
     if (!props) return false;

     for (const prop of Object.keys(filterProps)) {
         if (!props.hasOwnProperty(prop)) return false;

         if (!matchProps(props[prop], filterProps[prop], exactObjectMatch))
             return false;
     }

     return true;
}

I have worked out some possible ways to fix the problem in the library code, but this way seems the best for me, because it's pretty clear:

componentHasProps? No, because the props is null

I think this change will not break any use cases, but will make the library code safer in terms of various errors.

Solution details:

I am willing to take over the implementation and start the merge request according to the rules, if necessary. Also, this fix is needed as soon as possible due to the tight implementation deadline, so I have to ask for your help as soon as possible.

Dmitry-Ostashev commented 1 year ago

Thank you for your report and contribution. Could you please add a test to check the fixed behavior?

denisputnov commented 1 year ago

Hi, @Dmitry-Ostashev

The test for withProps method is already implemented here

I tried to add some test cases to test the corrected case, but it is unfortunately not reproducible for me still in the playground and I can't define the root cause for this problem in the my production code.