Skip to content

docs: point setup at .nvmrc and document how to run the tests - #1378

Open
ishan-one8 wants to merge 1 commit into
RocketChat:developfrom
ishan-one8:docs/setup-node-version-and-tests
Open

ishan-one8 wants to merge 1 commit into
RocketChat:developfrom
ishan-one8:docs/setup-node-version-and-tests

Conversation

@ishan-one8

Copy link
Copy Markdown

Point setup at .nvmrc and document how to run the tests

Who this is for: someone cloning EmbeddedChat for the first time and following the README top to bottom. Right now that person is told to install Node 22, and yarn then refuses to install anything.

Fixes #1377

Acceptance Criteria fulfillment

  • Prerequisites now name .nvmrc as the source of truth instead of a hard-coded major, and show nvm install / nvm use with no argument, which read it
  • The preinstall check is documented, including the error it prints and how to recover from it
  • A new Running the Tests section covers which packages have tests and how to run them
  • Every statement in the new text was verified against develop

The moment this documents

README.md says:

  • Node.js: Version 22 (LTS) is required.

while .nvmrc pins v24.0.0 and scripts/node-check.js enforces that major on preinstall. Following the README exactly is what breaks the install, which is a bad first ten minutes for a new contributor.

The second half is the thing I lost the most time to after that: nothing in the repo says how to run tests, and there is no CONTRIBUTING.md. Finding out that packages/layout_editor uses Node's built-in runner, and that a package's tests must be run from inside that package — otherwise Babel resolves its root config from the wrong cwd and never finds packages/react/babel.config.js — is currently rediscovered by reading package.json files one by one.

Video/Screenshots

Not applicable — documentation only.

PR Test Details

Each claim in the new text was checked against develop before writing it:

  • cd packages/layout_editor && yarn test → passes (node --test src/lib/*.test.js)
  • packages/e2e-react test script → playwright test
  • packages/react has no test script, and running its existing src/index.test.js through jest fails with "Cannot use import statement outside a module" — so the README now points at fix(react): src/index.test.js fails on develop due to missing swiper/element/bundle module #1191 rather than claiming it works
  • nvm install / nvm use with no argument read .nvmrc

No code changes, so nothing to build or lint beyond the docs themselves.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

docs: README says Node 22 is required, but .nvmrc pins v24 and preinstall enforces it

1 participant