packages/toolkit/src/react/ ships five public components and has no test file at
all. It is the only module in the package with zero tests.
The module is a published subpath export, @ocpp-debugkit/toolkit/react, so these
components are part of the public API:
SessionTimeline (components.tsx:97)
MessageInspector (components.tsx:131)
FailureSummary (components.tsx:187)
ReportViewer (components.tsx:232)
ReplayControls (components.tsx:251)
388 lines across components.tsx, index.ts and types.ts, none of it exercised.
The website consumes this module, so a regression here reaches production with
nothing to catch it.
Why this is not a good-first-issue
The module cannot be tested as the project stands, and fixing that needs decisions
rather than typing:
vitest.config.ts sets environment: 'node'. Rendering components needs a DOM,
so either a jsdom or happy-dom dependency plus a per-file
@vitest-environment pragma, or an environment override scoped to this directory.
- There is no component-testing library.
@testing-library/react is the
conventional choice, but it is a new dependency and a new idiom for this repo.
react and react-dom are peer dependencies, not dev dependencies, so they need
adding to devDependencies for tests to run.
ReportViewer renders caller-supplied HTML, so its test is partly a security
question about what the component is expected to sanitize, if anything. That is
worth settling deliberately rather than pinning current behaviour by accident.
Suggested shape
- Decide the DOM environment and scope it as narrowly as possible, so the rest of
the suite keeps running under node.
- Add
@testing-library/react and the React dev dependencies.
- Cover the presentational contract of each component: given props, the expected
text and structure appear. These components are documented as SSR-safe and
free of module-level window access, which is worth asserting too.
- Treat
ReportViewer separately and state explicitly what it does and does not
sanitize.
Once the approach is settled, individual per-component tests would make good
newcomer work, so this issue is worth splitting at that point.
Scope
Maintainer decision first, implementation second. No changeset: nothing in the
published output changes.
Found while surveying the repository for newcomer-suitable work.
packages/toolkit/src/react/ships five public components and has no test file atall. It is the only module in the package with zero tests.
The module is a published subpath export,
@ocpp-debugkit/toolkit/react, so thesecomponents are part of the public API:
SessionTimeline(components.tsx:97)MessageInspector(components.tsx:131)FailureSummary(components.tsx:187)ReportViewer(components.tsx:232)ReplayControls(components.tsx:251)388 lines across
components.tsx,index.tsandtypes.ts, none of it exercised.The website consumes this module, so a regression here reaches production with
nothing to catch it.
Why this is not a good-first-issue
The module cannot be tested as the project stands, and fixing that needs decisions
rather than typing:
vitest.config.tssetsenvironment: 'node'. Rendering components needs a DOM,so either a
jsdomorhappy-domdependency plus a per-file@vitest-environmentpragma, or an environment override scoped to this directory.@testing-library/reactis theconventional choice, but it is a new dependency and a new idiom for this repo.
reactandreact-domare peer dependencies, not dev dependencies, so they needadding to devDependencies for tests to run.
ReportViewerrenders caller-supplied HTML, so its test is partly a securityquestion about what the component is expected to sanitize, if anything. That is
worth settling deliberately rather than pinning current behaviour by accident.
Suggested shape
the suite keeps running under
node.@testing-library/reactand the React dev dependencies.text and structure appear. These components are documented as SSR-safe and
free of module-level
windowaccess, which is worth asserting too.ReportViewerseparately and state explicitly what it does and does notsanitize.
Once the approach is settled, individual per-component tests would make good
newcomer work, so this issue is worth splitting at that point.
Scope
Maintainer decision first, implementation second. No changeset: nothing in the
published output changes.
Found while surveying the repository for newcomer-suitable work.