matrix-org / matrix-org/matrix-js-sdk

Simplify use as a git dependency in create-react-app

Open
#3,015 2 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
TypeScript
Stars
2.2k
Forks
704
Avg merge
1d 20h
Merged PRs (30d)
40

Description

Installing matrix-js-sdk as a git dependency (e.g. for a fork of the repo) in certain browser dev environments – like create-react-app – breaks builds. (I don't think this issue is limited to create-react-app, but that is what I'm using.)

Repro
I can create a repo for this if it's helpful, but I'd prefer not to keep one around on my Github account.

  1. Create a new create-react-app project with TypeScript enabled, using yarn 1 as package manager: yarn create react-app matrix-js-sdk-git-dep-bug --template typescript
  2. Add matrix-js-sdk as git URL dependency: yarn add matrix-js-sdk@matrix-org/matrix-js-sdk
  3. yarn start

Project should build, but it has the following errors: https://gist.github.com/davidisaaclee/65a6d1a93b594b4d5e29fa3f352125a2

I think these are the causes:

  1. Update: Fixed in https://github.com/matrix-org/matrix-js-sdk/pull/3051 matrix-js-sdk's package.json#browser is set to ./lib/browser-index.ts, which afaict is not actually created by yarn build (it appears to be an intermediate artifact of yarn build:compile-web); so when looking for a browser entrypoint, one is not found (and so main is likely used, which causes (1) errors since src/index.ts requires transpilation but is not being transpiled, and (2) the kinds of errors that browser-index was created to solve, like undeclared global fields).
  2. matrix-js-sdk uses the prepublishOnly hook to transpile and emit type declarations. This hook is not called for dependencies specified using git URLs (https://github.com/npm/npm/issues/3055); so installing via git URL dependency never creates these necessary files.

I've solved these problems for my use case by doing the following (links to actual code changes included):

  1. Update: Fixed in https://github.com/matrix-org/matrix-js-sdk/pull/3051 Not fixed, see my comment below. Set package.json#browser to ./lib/browser-index.js, and add a package.json#types field pointing to ./lib/browser-index.d.ts. https://github.com/matrix-org/matrix-js-sdk/commit/5f9cb34a8307b64eba6d4ad5d31da855b2dd4875 https://github.com/matrix-org/matrix-js-sdk/commit/ef4f9f13c2f710dfe0ec660b4be7b2c7f1263782
  • I tried using ./dist/browser-matrix.js as browser, but with that, matrix-js-sdk's module at runtime was empty (i.e. import * as matrix from 'matrix-js-sdk' yielded Object.keys(matrix).length === 0; same for default import)
  • I see that element-web uses ./src/browser-index.ts (by way of matrix_src_browser), and passes matrix-js-sdk through Babel using its Webpack config. This isn't worth the extra configuration for my use case (especially difficult for create-react-app!); and I think the majority of people will not want to special-case matrix-js-sdk in their configs.
  • afaict, package.json#types does not have a way to export browser/Node-specific typings (a la main / browser). I am not using my fork in any Node projects, so this isn't an issue for me. I'm not sure if this would cause an issue for Node projects.
  1. Update: looks like I made a typo in the linked commit. I still think using prepack is a good approach. I'll address it if this issue gets any kind of traction. Change the prepublishOnly script to run instead at prepack (i.e. delete prepublishOnly hook in package.json, add "prepack": "yarn build") https://github.com/matrix-org/matrix-js-sdk/commit/972f256eb7b7343c13f66ec124e8a54d8b699260
  • prepare is another lifecycle hook that people use for this, but is not supported by Yarn, which I am using
  • I don't know all the places where prepack will run. It definitely runs when installing all dependencies in matrix-js-sdk (e.g. by calling yarn in the mjs project dir); that stinks, since this is unnecessary and yarn build can take some time.

I'd be happy to turn the commits above into one or two PRs, but I thought I'd write this issue first in case there are gotchas or alternatives I'm missing. (For example, I see that some of the mentioned package.json fields are being manipulated by release.sh.)

Thanks for the useful SDK!

Contributor guide

Open the contributing guide

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.

Research direction

Start with package.json, especially the browser, types, and lifecycle-script fields, then inspect release.sh as noted in the issue. Reproduce the failure with the yarn 1 create-react-app TypeScript commands and compare the resulting errors with the linked gist. Done means a matrix-js-sdk git dependency builds and runs in that environment without requiring project-specific configuration.

Written by the indexing model from the issue text.

Assessment

Tech stack
typescript
Domain
build-system, tooling
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.