scratch-paint's npm test does not run lint, so CI never lints paint; one lint error is currently in develop

Open Beginner friendly
  • Is something wrong?
#754 0 comments 0 reactions 0 assignees View on GitHub

Maintainers usually reply within 1 day

Nobody has claimed this yet.

Assessment

Difficulty
2/5
Estimated time
1-3 hours
Newbie friendliness
86/100
Issue type
Bug
Clarity
Clearly specified
Activity status
Active
Tech stack
eslint, javascript, node.js
Domain
ci-cd, testing

Research direction

Start with packages/scratch-paint/package.json and .github/actions/test-package/action.yml to confirm how package tests run in CI. Run npm run lint in packages/scratch-paint and fix the reported chained-call error in src/containers/paper-canvas.jsx:236. Then set the package's test script to run lint before unit tests; done means npm test runs both successfully.

Written by the indexing model from the issue text.

Description

Expected Behavior

npm test in packages/scratch-paint runs lint and then the unit tests, like every other package in the monorepo, so that CI (which runs npm run test per package) catches lint errors in paint.

Actual Behavior

packages/scratch-paint/package.json has

"lint": "eslint",
"test": "npm run unit",

so npm test runs only Jest. .github/actions/test-package/action.yml runs npm run test, so paint is the only package whose lint never runs in CI.

How it got this way: in the standalone scratch-paint repo, test was npm run lint && npm run unit until commit 433c0ae45f (2023-10-17, "ci: update package.json and package-lock"), which trimmed it to npm run unit at the same time as a deploy.yml change. That made sense while paint had its own workflow; after the monorepo import, where CI only runs each package's npm run test, the lint step silently disappeared.

As a result there is a lint error in develop today that CI did not catch. npm run lint in packages/scratch-paint reports:

src/containers/paper-canvas.jsx
  236:63  error  Expected line break before `.then`  @stylistic/newline-per-chained-call

The line is

getPaperSandbox().then(sandbox => sandbox.send({svg})).then(result => {

Steps to Reproduce

  1. Clone scratch-editor, check out develop, run npm ci from the repo root.
  2. cd packages/scratch-paint.
  3. Run npm test: lint does not run.
  4. Run npm run lint: see the newline-per-chained-call error in paper-canvas.jsx:236.

System Details

  • Observed on Windows 10 Pro 22H2 (build 19045), Node v24.21.0, npm 12.0.2; the missing lint step and the paper-canvas.jsx error are not platform-specific.
  • scratch-editor develop at 7b5559d29 (v15.3.0), fresh npm ci

Notes

Suggested fix in two steps so each commit stays small: first fix the one chained-call line, then change paint's test script to npm run lint && npm run unit. Neither changes published code.

Dominant language
JavaScript
Stars
310
Forks
299
Avg merge
19h 41m
Merged PRs (30d)
14

Getting set up

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from scratchfoundation/scratch-editor

All issues in scratchfoundation/scratch-editor

Similar issues

More JavaScript issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.