fix #283 - feat: Reduce warnings in build and test logs - #323
Conversation
Signed-off-by: fantonangeli <fabrizio.antonangeli@gmail.com>
Signed-off-by: fantonangeli <fabrizio.antonangeli@gmail.com>
Signed-off-by: fantonangeli <fabrizio.antonangeli@gmail.com>
Signed-off-by: fantonangeli <fabrizio.antonangeli@gmail.com>
Signed-off-by: fantonangeli <fabrizio.antonangeli@gmail.com>
Signed-off-by: fantonangeli <fabrizio.antonangeli@gmail.com>
Signed-off-by: fantonangeli <fabrizio.antonangeli@gmail.com>
Signed-off-by: fantonangeli <fabrizio.antonangeli@gmail.com>
Signed-off-by: fantonangeli <fabrizio.antonangeli@gmail.com>
Signed-off-by: fantonangeli <fabrizio.antonangeli@gmail.com>
Signed-off-by: fantonangeli <fabrizio.antonangeli@gmail.com>
…ce-warnings-in-build-and-test-logs Signed-off-by: fantonangeli <fabrizio.antonangeli@gmail.com>
✅ Deploy Preview for openworkflow-editor ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
There was a problem hiding this comment.
Pull request overview
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Reduces CI/build/test noise by addressing React/testing warnings, consolidating Storybook story setup, and tightening linting to fail on warnings.
Changes:
- Refactors Storybook stories to share a common
createWorkflowStoryhelper (incl. aplaystep to wait for render completion). - Adjusts tests/stories to minimize React
act(...)/render warnings (e.g., SVG edge rendering, disabling lint rule for spec fieldthen). - Introduces/updates oxlint + formatting config and makes some builds fail on lint warnings.
Reviewed changes
Copilot reviewed 18 out of 19 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/open-workflow-diagram-editor/vitest.config.ts | Simplifies dirname resolution for Vitest config. |
| packages/open-workflow-diagram-editor/tests/side-panel/NodeDetailsView.test.tsx | Suppresses unicorn/no-thenable for spec field then. |
| packages/open-workflow-diagram-editor/tests/react-flow/edges/Edges.test.tsx | Wraps edge components in <svg> and adjusts snapshot target to reduce warnings. |
| packages/open-workflow-diagram-editor/tests/core/taskDetails.test.ts | Suppresses unicorn/no-thenable for spec field then. |
| packages/open-workflow-diagram-editor/stories/use-cases/UseCases.stories.tsx | Uses shared story helper instead of local factory. |
| packages/open-workflow-diagram-editor/stories/helpers.ts | Adds shared createWorkflowStory helper with default args and play wait. |
| packages/open-workflow-diagram-editor/stories/features/ValidationErrors.stories.tsx | Uses shared story helper instead of local factory. |
| packages/open-workflow-diagram-editor/stories/features/DiagramEditorErrorBoundary.stories.tsx | Adds console.error filtering via spyOn to reduce noisy errors in story runs. |
| packages/open-workflow-diagram-editor/stories/features/DiagramEditorDragNDrop.tsx | Removes console logging. |
| packages/open-workflow-diagram-editor/stories/features/DiagramEditor.stories.tsx | Replaces inline story objects with shared helper usage. |
| packages/open-workflow-diagram-editor/stories/examples/Examples.stories.tsx | Uses shared story helper instead of local factory. |
| packages/open-workflow-diagram-editor/stories/README.md | Documents the new helper file. |
| packages/open-workflow-diagram-editor/src/components/ui/combobox.tsx | Removes unused children from input props destructure. |
| packages/i18n/package.json | Adds oxlint/oxfmt scripts and runs lint in build:prod. |
| packages/i18n/.oxlintrc.json | Extends root oxlint configuration. |
| packages/i18n/.oxfmtrc.json | Extends root oxfmt configuration. |
| examples/vanilla-web-component/vite.config.ts | Raises Vite chunk size warning limit. |
| .oxlintrc.json | Updates oxlint plugins and enables denyWarnings. |
Files not reviewed (1)
- pnpm-lock.yaml: Generated file
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Signed-off-by: fantonangeli <fabrizio.antonangeli@gmail.com>
Signed-off-by: fantonangeli <fabrizio.antonangeli@gmail.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 18 out of 19 changed files in this pull request and generated no new comments.
Files not reviewed (1)
- pnpm-lock.yaml: Generated file
Suppressed comments (2)
packages/open-workflow-diagram-editor/tests/react-flow/edges/Edges.test.tsx:216
- Using
svg!.firstChildis brittle becausefirstChildcan be a whitespace/text node depending on how JSX is compiled/rendered. Prefer snapshotting the first element child to avoid flakey/incorrect snapshots.
const svg = container.querySelector("svg");
expect(svg).not.toBeNull();
expect(svg!.firstChild).toMatchSnapshot();
packages/open-workflow-diagram-editor/stories/features/DiagramEditorErrorBoundary.stories.tsx:42
- Capturing
console.errorinto a variable can lose itsthisbinding in some runtimes. Binding it toconsoleavoids edge-case failures when forwarding to the original implementation from inside the spy.
const originalConsoleError = console.error;
Signed-off-by: fantonangeli <fabrizio.antonangeli@gmail.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 18 out of 19 changed files in this pull request and generated no new comments.
Files not reviewed (1)
- pnpm-lock.yaml: Generated file
Suppressed comments (1)
packages/open-workflow-diagram-editor/stories/features/DiagramEditorErrorBoundary.stories.tsx:52
- The
mockImplementationcontains an overly long condition with missing spacing around===, and it calls the extractedconsole.errorfunction without an explicit receiver. Formatting this improves readability and helps avoid potential lint failures; using.apply(console, ...)preserves the console method binding in all runtimes.
const consoleErrorSpy = spyOn(console, "error").mockImplementation((...args) => {
const error = args[1];
if (error instanceof Error && (error.message===DEFAULT_ERROR_MESSAGE || error.message===CUSTOM_ERROR_MESSAGE)) {
return;
This reverts commit 839f8de. Signed-off-by: fantonangeli <fabrizio.antonangeli@gmail.com>
Signed-off-by: fantonangeli <fabrizio.antonangeli@gmail.com>
213efee to
bc63334
Compare
open-workflow-specification#323 (comment) Signed-off-by: fantonangeli <fabrizio.antonangeli@gmail.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 18 out of 19 changed files in this pull request and generated no new comments.
Files not reviewed (1)
- pnpm-lock.yaml: Generated file
Suppressed comments (3)
packages/open-workflow-diagram-editor/stories/helpers.ts:20
DiagramEditoris imported usingimport type, but it’s then referenced in atypeoftype query (StoryObj<typeof DiagramEditor>). In TypeScript,typeofin a type position still requires the symbol to exist in the value namespace, so this pattern typically fails with “Cannot use 'DiagramEditor' as a value because it was imported using 'import type'”. Use a module type query instead (or importDiagramEditoras a value).
import type { DiagramEditor } from "./features/DiagramEditor";
type Story = StoryObj<typeof DiagramEditor>;
packages/open-workflow-diagram-editor/tests/core/taskDetails.test.ts:274
- Grammar nit in the newly added eslint-disable reason: “a Open” should be “an Open”.
// eslint-disable-next-line unicorn/no-thenable -- then is a Open Workflow Spec field
packages/open-workflow-diagram-editor/tests/side-panel/NodeDetailsView.test.tsx:39
- Grammar nit in the newly added eslint-disable reason: “a Open” should be “an Open”.
// eslint-disable-next-line unicorn/no-thenable -- then is a Open Workflow Spec field
open-workflow-specification#323 (review) Signed-off-by: fantonangeli <fabrizio.antonangeli@gmail.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 18 out of 19 changed files in this pull request and generated no new comments.
Files not reviewed (1)
- pnpm-lock.yaml: Generated file
Suppressed comments (1)
packages/open-workflow-diagram-editor/src/components/ui/combobox.tsx:276
ComboboxChipsInputno longer stripschildrenout of the props spread. If a caller passes children (even accidentally via composition), it will be forwarded intoComboboxPrimitive.Inputwhich likely renders an<input>; React will warn on children for void elements (and it can reintroduce the warnings this PR is trying to eliminate).
function ComboboxChipsInput({ className, ...props }: ComboboxPrimitive.Input.Props) {
return (
<ComboboxPrimitive.Input
data-slot="combobox-chip-input"
className={cn("dec:min-w-16 dec:flex-1 dec:outline-none", className)}
{...props}
/>
a496524
into
open-workflow-specification:main
Closes #283
Description
The open-workflow-diagram-editor build currently produces a large number of warnings, like for React
act(...), accessibility, and lint warnings.Example job: https://github.com/open-workflow-specification/editor/actions/runs/30353445942/job/90256050921?pr=282
Log snippet:
How to test:
pnpm build:prodOR open the CI logs from this PRMotivation:
Reducing the existing warnings would make CI logs easier to review and help new warnings to be discovered.
Notes:
This PR also introduces an oxlint configuration making the CI to fail on lint warnings