Developer experience improvements [?]

[jamacku]

• Dependabot to keep dependencies up to date
• Codecov to keep track of code coverage
• Yarn as package manager
• Updated structure of tsconfig
• Run workflow for node 16.x and 17.x as well

[mgrabovsky]

Sweet! Thanks a lot for these changes.

The build is currently failing on 'getState' is declared but its value is never read. I would prefer that the build not fail in these cases. Emitting a warning is fine (and I presume eslint will keep being noisy), but an error is a bit too annoying at this stage. Let's not be overly “pedantic” yet.

[jamacku]

Yeah, I prefer this setup, since it forces me to fix this issue. But I understand your point.

But still, I would highly recommend using these options in later stages of development.

[jamacku]

• Codecov to keep track of code coverage

@mgrabovsky, Just note, that you need to enable Codecov for this repo in order to make it work.

[mgrabovsky]

Huh, this error just doesn't make sense, does it? Or is there some secret trick for using icons in a typed environment?

TS2786: 'OutlinedThumbsUpIcon' cannot be used as a JSX component.
  Its instance type 'Component<SVGIconProps, any, any>' is not a valid JSX element.
    89 |     return (
    90 |         <Button key="waived" variant="tertiary" onClick={onClick}>
  > 91 |             <OutlinedThumbsUpIcon />
       |              ^^^^^^^^^^^^^^^^^^^^
    92 |             <span className={styles.waive}>waive</span>
    93 |         </Button>
    94 |     );

Also, is there a way to a) print out the locations of the error (filename + line number), and b) all the errors at once in the build workflow?

[jamacku]

As far as I know builds are isolated environments and there is no way to combine logs from them. But usually in a setup like this if multiple/all of them fail it's usually the same error.

Regarding the error, I have no clue, it seems to me like some React related error. It first occurred when I switched to yarn, but the package set seems to be the same.

[mgrabovsky]

As far as I know builds are isolated environments and there is no way to combine logs from them. But usually in a setup like this if multiple/all of them fail it's usually the same error.

Sorry, I was somewhat imprecise in that question. What I'm wondering is why is the output from tsc from the build script different from the start one. When I run yarn start, it prints out all the type errors at once with filenames, line number and all that. With yarn build in the CI here on GitHub, we only get one error and no indication where it comes from.

Regarding the error, I have no clue, it seems to me like some React related error. It first occurred when I switched to yarn, but the package set seems to be the same.

I suppose it could be a bug in Patternfly. I'll try to file a bug.

[jamacku]

Sorry, I was somewhat imprecise in that question. What I'm wondering is why is the output from tsc from the build script different from the start one. When I run yarn start, it prints out all the type errors at once with filenames, line number and all that. With yarn build in the CI here on GitHub, we only get one error and no indication where it comes from.

I think it is because yarn build is using react-scripts build from package.json. It looks similar to this issue: https://stackoverflow.com/a/57226138/10221282

OR: We can call npx tsc, but I thought that it would be a good idea to run the build script, to make it close to the "production" version.

[mgrabovsky]

Sorry for leaving this hanging for such a long time!

1. It seems like the yarn build problem has no easy solution, so let's just leave it as is for now.
2. I got a reply to the “cannot be used as a JSX component” issue. It looks like it should be enough to add some resolves to package.json. Could you please try that out?
3. We've updated some packages in the main branch in the meantime. Could you please rebase on top of that?

[jamacku]

I've added resolutions to package.json and I was able to run yarn build locally. @mgrabovsky PTAL

[jamacku]

CI fails because of:

Treating warnings as errors because process.env.CI = true.
Most CI servers set it automatically.

I would rather resolve the warnings then setting it to false, but it's up to you.