Fix annotations for pull requests #50

andershagbard · 2024-11-13T09:59:20Z

Make the sha the sha for the PR, and not the commit. This makes the annotations show up in "Files changed"
Made sure the annotation.path is relative to GitHub, and not the --path value
Use spread for ...ctx.repo

Finalize PR

andershagbard · 2024-11-13T10:07:10Z

We may also consider marking the job as completed and succeeded if using a token, as annotations will show up as an error.

Ignore the jobs in the middle, this is mixed with my other actions.

andershagbard · 2025-06-10T10:03:49Z

@aswamy Do you have any update on this? We really want to integrate this tool in our CI

aswamy

Hey @andershagbard, i was just trying to run this on a repo and had a question

aswamy · 2025-06-10T16:20:46Z

src/addAnnotations.ts

+    .flatMap((report) =>
      report.offenses.map((offense) => ({
-        path: path.relative(root, path.resolve(report.path)),
+        path: path.relative(cwd, path.resolve(report.path)),


Why is this not from the root anymore?

I think it was because the Shopify files are not necessarily in the root of the repo

Right. might have diff root, so src/snippets/foo.liquid instead of snippets/foo.liquid? Could be useful for multi-root setups, 👍

This reverts commit 26cbb4c.

andershagbard · 2025-06-15T11:13:36Z

Updated this PR. Threw out all non related to making the annotations work. Will put the other stuff into different PRs

charlespwd

ty!

aswamy · 2025-06-16T14:42:22Z

Tested out the latest change and looks like the action works as expected:

When the theme has a syntax errors, the theme check errors out
When the theme has a warning, the theme check passes with a warning
When the theme is nested in a repo, you can run theme check with a different root in CI

andershagbard · 2025-06-24T06:12:24Z

@charlespwd Did the dist build correctly?

I am getting these this error now:

/usr/bin/npm install --no-package-lock --no-save @shopify/cli
npm error Cannot read properties of null (reading 'matches')
npm error A complete log of this run can be found in: /home/runner/.npm/_logs/2025-0[6](https://github.com/ORG/REPO/actions/runs/1234/job/5678?pr=538#step:5:7)-23T14_28_04_196Z-debug-0.log
Error: The process '/usr/bin/npm' failed with exit code 1
    at ExecState._setResult (/home/runner/_work/_actions/shopify/theme-check-action/v2/dist/index.js:1:246[8](https://github.com/ORG/REPO/runs/1234/job/5678?pr=538#step:5:9)4)
    at ExecState.CheckComplete (/home/runner/_work/_actions/shopify/theme-check-action/v2/dist/index.js:1:24244)
    at ChildProcess.<anonymous> (/home/runner/_work/_actions/shopify/theme-check-action/v2/dist/index.js:1:23085)
Error: The process '/usr/bin/npm' failed with exit code 1
    at ChildProcess.emit (node:events:524:28)
    at maybeClose (node:internal/child_process:1[10](https://github.com/ORG/REPO/actions/runs/1234/job/5678?pr=538#step:5:11)4:16)
    at Socket.<anonymous> (node:internal/child_process:456:11)
    at Socket.emit (node:events:524:28)
    at Pipe.<anonymous> (node:net:343:12)

madsenmm · 2025-06-24T09:13:30Z

@charlespwd getting an error alike @andershagbard
Let me know if you want the full error message

npm error Cannot read properties of null (reading 'matches')
npm error A complete log of this run can be found in: /root/.npm/_logs/2025-06-24T08_56_45_265Z-debug-0.log
Error: The process '/root/actions-runner/_work/_tool/node/22.15.1/x64/bin/npm' failed with exit code 1
    at ExecState._setResult (/root/actions-runner/_work/_actions/shopify/theme-check-action/v2/dist/index.js:1:24684)
    at ExecState.CheckComplete (/root/actions-runner/_work/_actions/shopify/theme-check-action/v2/dist/index.js:1:24244)
    at ChildProcess.<anonymous> (/root/actions-runner/_work/_actions/shopify/theme-check-action/v2/dist/index.js:1:23085)
    at ChildProcess.emit (node:events:524:28)
    at maybeClose (node:internal/child_process:1[104](https://github.com/cybersikker/shopify/actions/runs/15845939450/job/44668025814#step:6:105):16)
    at ChildProcess._handle.onexit (node:internal/child_process:304:5)
Error: The process '/root/actions-runner/_work/_tool/node/22.15.1/x64/bin/npm' failed with exit code 1

charlespwd · 2025-07-03T15:50:30Z

Yeah please.

charlespwd · 2025-07-03T15:59:12Z

Seems to be from a dependency? What the hell? I transpiled without minifying and even then I couldn't find where we'd be doing matches on something that is null?

andershagbard · 2025-07-04T06:00:57Z

@charlespwd I think I've had this issue before. I had to use my worker (Linux) to run ncc. I made an action for it:

Example from my repo:

 - name: Compile assets
    run: pnpm build # Running the ncc command

  - name: Commit changes
    uses: EndBug/add-and-commit@v9
    with:
      default_author: github_actions

madsenmm · 2025-07-04T07:04:50Z

Yeah please.

Full workflow error

Run shopify/theme-check-action@v2
  with:
    theme_root: ./src
    flags: --fail-level warning
    token: ***
    base: main
  env:
    PNPM_HOME: /root/setup-pnpm/node_modules/.bin
/root/actions-runner/_work/_tool/node/22.15.1/x64/bin/npm install --no-package-lock --no-save @shopify/cli
npm warn ERESOLVE overriding peer dependency
npm warn While resolving: @jridgewell/[email protected]
npm warn Found: peer typescript@">=3.7.0" from @rollup/[email protected]
npm warn node_modules/.pnpm/@[email protected]/node_modules/@jridgewell/trace-mapping/node_modules/@rollup/plugin-typescript
npm warn   dev @rollup/plugin-typescript@"11.1.6" from @jridgewell/[email protected]
npm warn   node_modules/.pnpm/@[email protected]/node_modules/@jridgewell/trace-mapping
npm warn
npm warn Could not resolve dependency:
npm warn peer typescript@">=3.7.0" from @rollup/[email protected]
npm warn node_modules/.pnpm/@[email protected]/node_modules/@jridgewell/trace-mapping/node_modules/@rollup/plugin-typescript
npm warn   dev @rollup/plugin-typescript@"11.1.6" from @jridgewell/[email protected]
npm warn   node_modules/.pnpm/@[email protected]/node_modules/@jridgewell/trace-mapping
npm warn ERESOLVE overriding peer dependency
npm warn While resolving: @jridgewell/[email protected]
npm warn Found: peer eslint@"^7.0.0 || ^8.0.0" from @typescript-eslint/[email protected]
npm warn node_modules/.pnpm/@[email protected]/node_modules/@jridgewell/trace-mapping/node_modules/@typescript-eslint/eslint-plugin
npm warn   dev @typescript-eslint/eslint-plugin@"6.18.1" from @jridgewell/[email protected]
npm warn   node_modules/.pnpm/@[email protected]/node_modules/@jridgewell/trace-mapping
npm warn
npm warn Could not resolve dependency:
npm warn peer eslint@"^7.0.0 || ^8.0.0" from @typescript-eslint/[email protected]
npm warn node_modules/.pnpm/@[email protected]/node_modules/@jridgewell/trace-mapping/node_modules/@typescript-eslint/eslint-plugin
npm warn   dev @typescript-eslint/eslint-plugin@"6.18.1" from @jridgewell/[email protected]
npm warn   node_modules/.pnpm/@[email protected]/node_modules/@jridgewell/trace-mapping
npm warn ERESOLVE overriding peer dependency
npm warn While resolving: @jridgewell/[email protected]
npm warn Found: peer @typescript-eslint/parser@"^6.0.0 || ^6.0.0-alpha" from @typescript-eslint/[email protected]
npm warn node_modules/.pnpm/@[email protected]/node_modules/@jridgewell/trace-mapping/node_modules/@typescript-eslint/eslint-plugin
npm warn   dev @typescript-eslint/eslint-plugin@"6.18.1" from @jridgewell/[email protected]
npm warn   node_modules/.pnpm/@[email protected]/node_modules/@jridgewell/trace-mapping
npm warn
npm warn Could not resolve dependency:
npm warn peer @typescript-eslint/parser@"^6.0.0 || ^6.0.0-alpha" from @typescript-eslint/[email protected]
npm warn node_modules/.pnpm/@[email protected]/node_modules/@jridgewell/trace-mapping/node_modules/@typescript-eslint/eslint-plugin
npm warn   dev @typescript-eslint/eslint-plugin@"6.18.1" from @jridgewell/[email protected]
npm warn   node_modules/.pnpm/@[email protected]/node_modules/@jridgewell/trace-mapping
npm warn ERESOLVE overriding peer dependency
npm warn While resolving: @jridgewell/[email protected]
npm warn Found: peer eslint@"^7.0.0 || ^8.0.0" from @typescript-eslint/[email protected]
npm warn node_modules/.pnpm/@[email protected]/node_modules/@jridgewell/trace-mapping/node_modules/@typescript-eslint/parser
npm warn   dev @typescript-eslint/parser@"6.18.1" from @jridgewell/[email protected]
npm warn   node_modules/.pnpm/@[email protected]/node_modules/@jridgewell/trace-mapping
npm warn   1 more (@typescript-eslint/eslint-plugin)
npm warn
npm warn Could not resolve dependency:
npm warn peer eslint@"^7.0.0 || ^8.0.0" from @typescript-eslint/[email protected]
npm warn node_modules/.pnpm/@[email protected]/node_modules/@jridgewell/trace-mapping/node_modules/@typescript-eslint/parser
npm warn   dev @typescript-eslint/parser@"6.18.1" from @jridgewell/[email protected]
npm warn   node_modules/.pnpm/@[email protected]/node_modules/@jridgewell/trace-mapping
npm warn   1 more (@typescript-eslint/eslint-plugin)
npm warn ERESOLVE overriding peer dependency
npm warn ERESOLVE overriding peer dependency
npm warn While resolving: @ampproject/[email protected]
npm warn Found: peer @typescript-eslint/parser@"^5.0.0" from @typescript-eslint/[email protected]
npm warn node_modules/.pnpm/@[email protected]/node_modules/@ampproject/remapping/node_modules/@typescript-eslint/eslint-plugin
npm warn   dev @typescript-eslint/eslint-plugin@"5.20.0" from @ampproject/[email protected]
npm warn   node_modules/.pnpm/@[email protected]/node_modules/@ampproject/remapping
npm warn
npm warn Could not resolve dependency:
npm warn peer @typescript-eslint/parser@"^5.0.0" from @typescript-eslint/[email protected]
npm warn node_modules/.pnpm/@[email protected]/node_modules/@ampproject/remapping/node_modules/@typescript-eslint/eslint-plugin
npm warn   dev @typescript-eslint/eslint-plugin@"5.20.0" from @ampproject/[email protected]
npm warn   node_modules/.pnpm/@[email protected]/node_modules/@ampproject/remapping
npm warn ERESOLVE overriding peer dependency
npm warn ERESOLVE overriding peer dependency
npm warn ERESOLVE overriding peer dependency
npm warn While resolving: @jridgewell/[email protected]
npm warn Found: peer @typescript-eslint/parser@"^5.0.0" from @typescript-eslint/[email protected]
npm warn node_modules/.pnpm/@[email protected]/node_modules/@jridgewell/gen-mapping/node_modules/@typescript-eslint/eslint-plugin
npm warn   dev @typescript-eslint/eslint-plugin@"5.21.0" from @jridgewell/[email protected]
npm warn   node_modules/.pnpm/@[email protected]/node_modules/@jridgewell/gen-mapping
npm warn
npm warn Could not resolve dependency:
npm warn peer @typescript-eslint/parser@"^5.0.0" from @typescript-eslint/[email protected]
npm warn node_modules/.pnpm/@[email protected]/node_modules/@jridgewell/gen-mapping/node_modules/@typescript-eslint/eslint-plugin
npm warn   dev @typescript-eslint/eslint-plugin@"5.21.0" from @jridgewell/[email protected]
npm warn   node_modules/.pnpm/@[email protected]/node_modules/@jridgewell/gen-mapping
npm warn ERESOLVE overriding peer dependency
npm warn While resolving: [email protected]
npm warn Found: peer svelte@"^3.2.0 || ^4.0.0-next.0 || ^5.0.0-next.0" from [email protected]
npm warn node_modules/.pnpm/[email protected]_@[email protected][email protected][email protected]/node_modules/prettier-plugin-tailwindcss/node_modules/prettier-plugin-svelte
npm warn   dev prettier-plugin-svelte@"^3.1.2" from [email protected]
npm warn   node_modules/.pnpm/[email protected]_@[email protected][email protected][email protected]/node_modules/prettier-plugin-tailwindcss
npm warn
npm warn Could not resolve dependency:
npm warn peer svelte@"^3.2.0 || ^4.0.0-next.0 || ^5.0.0-next.0" from [email protected]
npm warn node_modules/.pnpm/[email protected]_@[email protected][email protected][email protected]/node_modules/prettier-plugin-tailwindcss/node_modules/prettier-plugin-svelte
npm warn   dev prettier-plugin-svelte@"^3.1.2" from [email protected]
npm warn   node_modules/.pnpm/[email protected]_@[email protected][email protected][email protected]/node_modules/prettier-plugin-tailwindcss
npm error Cannot read properties of null (reading 'matches')
npm error A complete log of this run can be found in: /root/.npm/_logs/2025-06-24T11_13_38_759Z-debug-0.log
Error: The process '/root/actions-runner/_work/_tool/node/22.15.1/x64/bin/npm' failed with exit code 1
    at ExecState._setResult (/root/actions-runner/_work/_actions/shopify/theme-check-action/v2/dist/index.js:1:24684)
    at ExecState.CheckComplete (/root/actions-runner/_work/_actions/shopify/theme-check-action/v2/dist/index.js:1:24244)
    at ChildProcess.<anonymous> (/root/actions-runner/_work/_actions/shopify/theme-check-action/v2/dist/index.js:1:23085)
    at ChildProcess.emit (node:events:524:28)
    at maybeClose (node:internal/child_process:1104:16)
    at ChildProcess._handle.onexit (node:internal/child_process:304:5)
Error: The process '/root/actions-runner/_work/_tool/node/22.15.1/x64/bin/npm' failed with exit code 1

charlespwd · 2025-07-07T13:37:10Z

What the hell so it's the npm install that is causing issue? This is weird because we're running this in both Horizon and Dawn and the CI is green... Wondering if it has to do with some kind of conflict with a package.json that y'all have?

Maybe I need more flags? Something like don't install dev deps of CLI or somethign? Probably why it complains about svelte peer dependencies?

Hard to fix this if I can't reproduce. Can you try adding a step in your CI jobs that remove the package.json before running this command?

madsenmm · 2025-07-07T14:18:04Z

What the hell so it's the npm install that is causing issue? This is weird because we're running this in both Horizon and Dawn and the CI is green... Wondering if it has to do with some kind of conflict with a package.json that y'all have?

Maybe I need more flags? Something like don't install dev deps of CLI or somethign? Probably why it complains about svelte peer dependencies?

Hard to fix this if I can't reproduce. Can you try adding a step in your CI jobs that remove the package.json before running this command?

It works as expected when removing the package.json 😅
I just added:

- name: Remove package.json
   run: rm package.json

andershagbard · 2025-07-14T15:52:55Z

I think this is related to pnpm usage.

If I just install @shopify/cli in my repo using pnpm it works, but if I use npm it gives this error.

Should we give #42 a go again?

andershagbard and others added 21 commits November 12, 2024 14:56

Add logs

43c410d

Update

227d763

Update

ac1d454

Update

f3b65d6

Update

cbc7ec7

Fix

4254135

Update

f026bd3

Update

e55f861

Update

4b08459

UPDATE

af07df8

Update

0f29a1b

Update

ca65bad

Fix

d477157

Revert

a1df2d1

Test

c9a27da

Update

dfbeaec

Update

c5a2ac1

Add prettier

6874ff3

Update index.js

de89a97

Update

4b31d3a

Merge pull request #1 from andershagbard/log-test

16d8acc

Finalize PR

andershagbard added 3 commits November 13, 2024 11:14

Minimize runs

d20e7d3

Update addAnnotations.ts

c57a28a

Run build

3f8ae4c

andershagbard marked this pull request as ready for review November 13, 2024 10:18

aswamy self-assigned this Mar 20, 2025

aswamy reviewed Jun 10, 2025

View reviewed changes

aswamy approved these changes Jun 12, 2025

View reviewed changes

andershagbard added 3 commits June 15, 2025 12:38

Revert more

99115f8

More revert

c40b7be

Revert

4482f26

andershagbard changed the title ~~Fix annotations for PR, Prettier, and more~~ Fix annotations for pull requests Jun 15, 2025

andershagbard added 7 commits June 15, 2025 12:45

Update packages

26cbb4c

Revert "Update packages"

cbc9585

This reverts commit 26cbb4c.

Revert packages

158ba41

Revert more

85a1423

Revert

01123fb

Update yarn.lock

486431f

Build file

8492181

andershagbard marked this pull request as ready for review June 15, 2025 11:13

andershagbard requested a review from charlespwd June 15, 2025 11:13

This was referenced Jun 15, 2025

Simplify TypeScript #58

Open

Add Prettier #59

Open

charlespwd approved these changes Jun 16, 2025

View reviewed changes

aswamy merged commit b9ee009 into Shopify:main Jun 16, 2025
1 check passed

andershagbard deleted the fix-annotations branch June 16, 2025 15:47

charlespwd pushed a commit that referenced this pull request Jun 16, 2025

Fix annotations for pull requests (#50)

4f064ed

Fix annotations for pull requests #50

Fix annotations for pull requests #50

Uh oh!

Conversation

andershagbard commented Nov 13, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

andershagbard commented Nov 13, 2024

Uh oh!

andershagbard commented Jun 10, 2025

Uh oh!

aswamy left a comment

Choose a reason for hiding this comment

Uh oh!

aswamy Jun 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

andershagbard Jun 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

charlespwd Jun 12, 2025

Choose a reason for hiding this comment

Uh oh!

andershagbard commented Jun 15, 2025

Uh oh!

charlespwd left a comment

Choose a reason for hiding this comment

Uh oh!

aswamy commented Jun 16, 2025

Uh oh!

Uh oh!

andershagbard commented Jun 24, 2025

Uh oh!

madsenmm commented Jun 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

charlespwd commented Jul 3, 2025

Uh oh!

charlespwd commented Jul 3, 2025

Uh oh!

andershagbard commented Jul 4, 2025

Uh oh!

madsenmm commented Jul 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

charlespwd commented Jul 7, 2025

Uh oh!

madsenmm commented Jul 7, 2025

Uh oh!

andershagbard commented Jul 14, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

andershagbard commented Nov 13, 2024 •

edited

Loading

aswamy Jun 10, 2025 •

edited

Loading

andershagbard Jun 11, 2025 •

edited

Loading

madsenmm commented Jun 24, 2025 •

edited

Loading

madsenmm commented Jul 4, 2025 •

edited

Loading