ci: pin matrix-js-sdk to lockfile commit to avoid layered.sh clone

[liviaUeno]

fix(build): pin matrix-js-sdk to lockfile commit to avoid layered.sh clone

Context

The Build workflow was failing on every push to develop and on every PR (jobs Build on macos-14 and Build on windows-2022). Last broken run on develop: https://github.com/Buzzlabs/element-web/actions/runs/34639681774

Error:

ModuleNotFoundError: Can't resolve 'matrix-js-sdk/src/oidc/authorize' in 'apps/web/src/utils/oidc'

Cause

The Fetch layered build step runs scripts/layered.sh, which — when apps/web/package.json pins matrix-js-sdk to
github:matrix-org/matrix-js-sdk#develop — clones the upstream SDK's develop branch and links it in place of the locked dependency. Upstream has since moved/removed src/oidc/authorize.ts, so the fork's code (written against the commit pinned in the lockfile) no longer resolves against it.

The lockfile itself was already resolving matrix-js-sdk to a working commit (33e4db98) — that's why pnpm install --frozen-lockfile (what Buzzlabs CD uses, without layered.sh) has always passed. Only the Build workflow's extra layered.sh step, which explicitly re-fetches the SDK's latest develop when the version isn't pinned, was affected.

Fix

layered.sh already has a check for this case — if the SDK dependency isn't pinned to #develop, it skips the fetch/link step entirely:

if [ "$js_sdk_dep" = "github:matrix-org/matrix-js-sdk#develop" ]; then
    # clones and links upstream develop
else
    echo "Skipping matrix-js-sdk fetch and link as package.json pins $js_sdk_dep"
fi

So instead of touching the workflow, this PR pins matrix-js-sdk in
apps/web/package.json to the same commit already resolved in the lockfile (github:matrix-org/matrix-js-sdk#33e4db98814edb1e100ac8b9721cf84de998f429),
and regenerates pnpm-lock.yaml to match. layered.sh now takes the "skip" branch, so Build no longer depends on upstream's develop state.

How this was verified

Ran ./scripts/layered.sh locally after the change:

Skipping matrix-js-sdk fetch and link as package.json pins github:matrix-org/matrix-js-sdk#33e4db98814edb1e100ac8b9721cf84de998f429

Confirms the clone is skipped and pnpm install --frozen-lockfile succeeds with the updated lockfile.

Done when

Build is green on develop (macOS and Windows jobs) and on this PR.

[zZMathSP]

Thanks for this, Livs. The write-up is genuinely good: you found the real cause (layered.sh re-fetching upstream develop), read the script, and picked the smallest lever that flips its behaviour. That is exactly the right instinct, so keep it.

There is one lesson hiding in this PR that I want to make explicit, because it will come up again and again in your career:

A one-line change to a dependency can change the shape of node_modules, and lots of tooling silently depends on that shape.

Before the pin, layered.sh cloned the SDK into ./matrix-js-sdk and pnpm linked it. That produced a symlink at node_modules/matrix-js-sdk pointing at a real git checkout with its own node_modules inside. After the pin, layered.sh takes the skip branch and pnpm installs the SDK from a tarball, which lives at apps/web/node_modules/.pnpm/matrix-js-sdk@https+++codeload.../. Same package, same commit, completely different location on disk. Several config files in this repo were written against the old location, and they broke. I have left an inline comment for each one.

The practical takeaway: when you touch how a dependency is installed, the verification step is not "does pnpm install succeed", it is "does every workflow that runs this install still pass". On this PR that means the Build workflow (which you fixed), plus Jest, Docs and the TypeScript check. The PR's own CI run already shows two of those going red: https://github.com/Buzzlabs/element-web/actions/runs/36009268211. Always scroll through the whole checks list on your PR, not just the one you were targeting.

None of this is hard to fix, and the direction of the PR is right. Let's get Jest and Docs green and then talk about whether we want to make the "this fork never links a live SDK clone" decision explicit in one place instead of scattered across scripts. Ping me if any of the inline comments is unclear.

[zZMathSP]

1. This pin breaks every Jest suite (confirmed on the PR's CI).

Look at the Jest (Element Web) (1) job on this PR: Test Suites: 357 failed, 357 total / Tests: 0 total, and the first error is

SyntaxError: Cannot use import statement outside a module
  import { secureRandomString } from "matrix-js-sdk/src/randomstring"

On develop two weeks ago the same job was 13 failed / 344 passed, so this is new.

Why does it happen? The fork imports the SDK's TypeScript sources (matrix-js-sdk/src/...), so Jest must transpile them. Jest decides what to transpile using transformIgnorePatterns in apps/web/jest.config.ts#L51. That pattern says "ignore everything under node_modules/.pnpm/ except this allowlist". Before your change the SDK was a symlink to a git checkout outside .pnpm, so the pattern never matched it and it got transformed. Now the SDK lives inside .pnpm/matrix-js-sdk@https+++codeload.../, the pattern matches, and Jest hands raw TypeScript to Node.

Fix: add matrix-js-sdk to that negative-lookahead allowlist (the .pnpm directory name starts with matrix-js-sdk@, so the plain name is enough). Then run pnpm test locally in apps/web before pushing. Watching a test count go from 0 to ~340 passing is the kind of evidence you want to quote in the PR description.

[zZMathSP]

2. The Docs job now fails with ENOENT (also confirmed on the PR's CI).

The Docs job on this PR dies with:

Error: ENOENT: no such file or directory, open '.../element-web/node_modules/matrix-js-sdk/package.json'

right after the Skipping matrix-js-sdk fetch and link line from layered.sh. The same job passed on develop.

The culprit is docs/generated/[id].paths.ts#L14, which asks the mermaid generator to scan <repo-root>/node_modules/matrix-js-sdk. That path only ever existed because layered.sh ran pnpm link ./matrix-js-sdk at the workspace root. The root package.json does not depend on the SDK, so with the pin the SDK is only installed under apps/web/node_modules.

Fix options, simplest first: point it at apps/web/node_modules/matrix-js-sdk, or drop the SDK from the mermaid generation entirely if we don't care about diagramming upstream's workflows in our fork. Either way, run pnpm docs:build locally to confirm.

A useful habit: when you grep the repo for the thing you are changing, grep for its side effects too. Here, grep -rn "node_modules/matrix-js-sdk" would have surfaced both this file and the Playwright tsconfig below in seconds.

[zZMathSP]

3. Two smaller things that the pin quietly changes.

a) The matrix-js-sdk-sha workflow input is now a no-op. Both build-and-test.yaml#L75 and tests.yml#L56 forward that input as JS_SDK_GITHUB_BASE_REF, but layered.sh#L29 only reads it inside the #develop branch of the if, which we never enter any more. So a caller could pass a SHA, get a green build against 33e4db98, and never learn their SHA was ignored. Silent no-ops are worse than errors. Either remove/deprecate the input in the fork, or make layered.sh exit 1 when JS_SDK_GITHUB_BASE_REF is set but the dependency is pinned.

b) apps/web/playwright/tsconfig.json#L19 includes ../node_modules/matrix-js-sdk/node_modules/@matrix-org/olm/index.d.ts. A nested node_modules inside the SDK only exists for a linked git checkout. With pnpm's tarball layout the SDK's dependencies are siblings under .pnpm/<sdk>/node_modules/, so that include resolves to nothing. It is masked today because the TypeScript check already fails on develop for unrelated reasons, but it will bite us the moment that job is fixed. Resolving olm's types by package name (via types or paths) instead of a hard-coded nested path would be more robust.

Neither of these blocks the PR on its own. I am flagging them so you see the full blast radius of a one-line dependency change, and so we can decide together whether to fix them here or in a follow-up.

[zZMathSP]

This hunk is the entire semantic change to the lockfile, and it is correct: the specifier now matches package.json, and the version line below it was already resolving to 33e4db98. Nice confirmation that your diagnosis was right (the lock was fine, only layered.sh was overriding it).

[zZMathSP]

Unrelated churn from a full re-resolve.

This deprecated: line on jsrsasign@11.1.3 has nothing to do with the SDK pin. It appeared because a full pnpm install refreshed registry metadata for every package, not just the one you changed.

Why care about one line? Our pnpm-lock.yaml is shared with upstream element-hq/element-web, and every extra hunk we carry is a potential conflict the next time we merge from them. The habit to build: after regenerating a lockfile, read the diff and ask "can I explain every hunk from my change?". If not, either revert the stray hunk or call it out explicitly in the PR description so the reviewer knows you saw it.

Here I would simply revert this hunk; the specifier change above is all that is needed.