Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
liviaUeno wants to merge 6 commits into
base: develop
Choose a base branch
from
Open

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
32679c8
fix: pin matrix-js-sdk to lockfile commit to avoid layered.sh clone
liviaUeno Sep 24, 2026
e7bc13c
fix: transform matrix-js-sdk from the pnpm tarball layout
liviaUeno Oct 9, 2026
28c6671
fix: stop scanning matrix-js-sdk for workflows
liviaUeno Oct 9, 2026
4061e45
fix: fail when JS_SDK_GITHUB_BASE_REF is set but matrix-js-sdk is pinned
liviaUeno Oct 13, 2026
8ebbbc9
fix: drop unrelated jsrsasign deprecated note
liviaUeno Oct 13, 2026
4c15406
Merge remote-tracking branch 'origin/develop' into fix-remove-layered…
liviaUeno Oct 22, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion apps/web/jest.config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ const config: Config = {
"@vector-im/compound-web": "<rootDir>/node_modules/@vector-im/compound-web",
},
transformIgnorePatterns: [
`${path.join(__dirname, "../..")}/node_modules/.pnpm/(?!(mime|uuid|p-retry|is-network-error|react-merge-refs|is-ip|ip-regex|super-regex|function-timeout|time-span|convert-hrtime|clone-regexp|is-regexp|matrix-web-i18n|await-lock|@element-hq/web-shared-components|react-virtuoso|lodash|domutils|domhandler|domelementtype|dom-serializer|entities)).+$`,
`${path.join(__dirname, "../..")}/node_modules/.pnpm/(?!(mime|uuid|p-retry|is-network-error|react-merge-refs|is-ip|ip-regex|super-regex|function-timeout|time-span|convert-hrtime|clone-regexp|is-regexp|matrix-web-i18n|await-lock|@element-hq/web-shared-components|react-virtuoso|lodash|domutils|domhandler|domelementtype|dom-serializer|entities|matrix-js-sdk)).+$`,
],
collectCoverageFrom: [
"<rootDir>/src/**/*.{js,ts,tsx}",
Expand Down
2 changes: 1 addition & 1 deletion apps/web/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@
"lodash": "npm:lodash-es@4.18.1",
"maplibre-gl": "^5.0.0",
"matrix-encrypt-attachment": "^1.0.3",
"matrix-js-sdk": "github:matrix-org/matrix-js-sdk#develop",
"matrix-js-sdk": "github:matrix-org/matrix-js-sdk#33e4db98814edb1e100ac8b9721cf84de998f429",

@zZMathSP zZMathSP Sep 24, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@liviaUeno liviaUeno Oct 9, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done: e7bc13c

@zZMathSP zZMathSP Sep 24, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@liviaUeno liviaUeno Oct 9, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done: 28c6671

@zZMathSP zZMathSP Sep 24, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@liviaUeno liviaUeno Oct 13, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

3a Done: 4061e45

For 3b, I suggest fixing it together with the TypeScript check, since it can only be validated once that job passes again. I have registered it on the Basecamp card “[CI] Static Analysis quebrado: TypeScript”, so it will be addressed there (resolving olm types by package name instead of the nested path).

@liviaUeno liviaUeno Oct 23, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

3b Done: c58c695, in fix-crypto-wasm-types.

Instead of resolving olm's types by package name, I removed the dead include from apps/web/playwright/tsconfig.json: olm is not installed with the pinned SDK, and tsc -p playwright/tsconfig.json reports 0 errors before and after, so nothing needs those types. I can switch to the paths/types approach if you prefer.

That branch also declares @matrix-org/matrix-sdk-crypto-wasm as a devDependency of apps/web, which fixes the 4 remaining tsc errors inside the SDK. With it, pnpm -r --workspace-concurrency=1 lint:types passes locally.

"matrix-widget-api": "^1.16.1",
"memoize-one": "^6.0.0",
"mime": "^4.0.4",
Expand Down
2 changes: 1 addition & 1 deletion docs/generated/[id].paths.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ export default {
return [
{
params: { id: "automations" },
content: await genWorkflowMermaid([root, join(root, "node_modules", "matrix-js-sdk")]),
content: await genWorkflowMermaid([root]),
},
];
},
Expand Down
2 changes: 1 addition & 1 deletion pnpm-lock.yaml
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions scripts/layered.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,5 +36,9 @@ if [ "$js_sdk_dep" = "github:matrix-org/matrix-js-sdk#develop" ]; then
pnpm -C apps/web link ./matrix-js-sdk
pnpm link ./matrix-js-sdk
else
if [ -n "$JS_SDK_GITHUB_BASE_REF" ]; then
echo "ERROR: JS_SDK_GITHUB_BASE_REF ($JS_SDK_GITHUB_BASE_REF) was requested, but package.json pins matrix-js-sdk to $js_sdk_dep, so it would be ignored." >&2
exit 1
fi
echo "Skipping matrix-js-sdk fetch and link as package.json pins $js_sdk_dep"
fi
Loading