oscal-react-library icon indicating copy to clipboard operation
oscal-react-library copied to clipboard

onRestError failures hidden

Open tuckerzp opened this issue 3 years ago • 3 comments
trafficstars

Also, it looks like the failure above highlights an issue with the onRestError prop that we're using... somewhere we're failing to actually pass a function.

FAIL src/components/OSCALJsonEditor.test.js
  ● OSCALJsonEditor › should render as expected

    TypeError: props.onRestError is not a function



      at src/components/OSCALControlProse.js:605:23
      at src/components/oscal-utils/OSCALRestUtils.js:153:13

  ● OSCALJsonEditor › should render as expected

    TypeError: props.onRestError is not a function



      at src/components/OSCALControlProse.js:605:23
      at src/components/oscal-utils/OSCALRestUtils.js:153:13

  ● OSCALJsonEditor › should render as expected

    TypeError: props.onRestError is not a function



      at src/components/OSCALControlProse.js:605:23
      at src/components/oscal-utils/OSCALRestUtils.js:153:13

  ● OSCALJsonEditor › should render as expected

    TypeError: props.onRestError is not a function



      at src/components/OSCALControlProse.js:605:23
      at src/components/oscal-utils/OSCALRestUtils.js:153:13

  ● OSCALJsonEditor › should return JSON after clicking save

    connect ECONNREFUSED 127.0.0.1:80



  ● OSCALJsonEditor › should return JSON after clicking save

    connect ECONNREFUSED 127.0.0.1:80



  ● OSCALJsonEditor › should return JSON after clicking save

    connect ECONNREFUSED 127.0.0.1:80



  ● OSCALJsonEditor › should return JSON after clicking save

    connect ECONNREFUSED 127.0.0.1:80

This probably would have been prevented with stronger type checking. In any case, if it's trivial, we can fix it here. Otherwise, we should we probably open a separate issue.

Originally posted by @kylelaker in https://github.com/EasyDynamics/oscal-react-library/issues/581#issuecomment-1224895904

tuckerzp avatar Aug 24 '22 20:08 tuckerzp

Must specify React component property types before we can size or work this.

brian-ruf-ezd avatar Apr 18 '23 14:04 brian-ruf-ezd

@kylelaker do we have an issue for specifying React component property types that we can cite in the comment above?

brian-ruf-ezd avatar May 01 '23 15:05 brian-ruf-ezd

It looks like the logs we originally cited this from are gone. I agree with @kylelaker's original thought that progressively switching toTypeScript will lead to this issue being fixed.

If I had to guess, switching OSCALJsonEditor.test.js --> OSCALJsonEditor.test.tsx might be a good starting point. But it may not be worth the effort to actively try to replicate this bug if it just effects the tests and will naturally be fixed anyways.

do we have an issue for specifying React component property types that we can cite in the comment above?

I don't think we do but I also don't think adding one would provide a ton of value. So far, we have just been adding property type specifications when we switch files to TypeScript. Rather than having one big issue that touches the entire code base, it has made more sense to just use the boy scout rule when we make changes in various files.

tuckerzp avatar May 01 '23 15:05 tuckerzp