Skip to content

Commit

Permalink
deps: Upgrade to Node v16, in setup docs and in CI
Browse files Browse the repository at this point in the history
I've been using this for local development for a few weeks now,
and all seems well.  (That's longer than it needs to be; I just
hadn't come back to this to follow up.)

Node v12 is going EOL in just over a week, 2022-04-30, so it's
past time to make this upgrade official.

Also delete an old troubleshooting entry for a problem with Node v11
that was avoided by sticking to v10; and fix two other references to
Node v10 (where the problem was using an older version like v8) so
that they remain accurate now and for further upgrades in the future.

In particular, where we had been checking in our tests for Node v10+
(but in fact for >=10 and <20), just replace it with handy metadata
in our package.json to make Yarn (or `npm`) complain at install time.
Thanks to Anders Kaseorg for pointing out that package.json feature.

Fixes: #4263
  • Loading branch information
gnprice committed Apr 27, 2022
1 parent ec13701 commit 70cf310
Show file tree
Hide file tree
Showing 4 changed files with 9 additions and 52 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ jobs:
- name: Set up Node
uses: actions/setup-node@v2
with:
node-version: '12'
node-version: '16'
check-latest: true

- name: Run yarn install
Expand Down
39 changes: 5 additions & 34 deletions docs/howto/build-run.md
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ details to worry about.
Before starting, install these dependencies if you don't have them:
* [Git](https://git-scm.com/)
* [Node.js](https://nodejs.org/en/download/package-manager/): use the
**latest 12.x** version, not 14.x or later
**latest 16.x** version, or a later LTS version
* [Yarn](https://yarnpkg.com/en/docs/install), latest stable version

Then, run the commands below in your terminal:
Expand Down Expand Up @@ -150,37 +150,6 @@ Apart from the steps mentioned below, you may find the
[React Native troubleshooting docs]: https://reactnative.dev/docs/troubleshooting


### `yarn install` failure, at `fsevents`

When running `yarn install` on initial setup, if you see an error like
this:
```
warning Error running install script for optional dependency: "[...]/zulip-mobile/node_modules/fsevents: Command failed.
Exit code: 1
Command: node install
Arguments:
Directory: [...]/zulip-mobile/node_modules/fsevents
Output:
[... lots of output ...]
../../nan/nan_maybe_43_inl.h:112:15: error: no member named 'ForceSet' in 'v8::Object'
return obj->ForceSet(isolate->GetCurrentContext(), key, value, attribs);
~~~ ^ return obj->ForceSet(isolate->GetCurrentContext(), key, value, attribs);
[... lots more output ...]
node-pre-gyp ERR! not ok
Failed to execute [...]
```
then this is a known error caused by using Node 11, which one of our
dependencies (`fsevents`) isn't yet compatible with.

To fix the problem, use Node 10.x instead.

The same problem has also been observed when using Node 10 on commits that were
made when we were using Node 8, prior to Greg's recommendation to switch to Node
10 in 4e5e31ac2. To fix the problem in that case, use Node 8.


### `yarn install` failure about "Detox"

This should only happen when building old versions of the app, from
Expand Down Expand Up @@ -680,8 +649,10 @@ This can happen if you're using an older version of Node, such as
Node 8. (Probably this means our Jest config doesn't have Babel set up
quite right. Discussion [here][jest-babel-discussion].)

To fix this, use Node 10.x instead. You can check what version is
installed by running the command `node --version`.
To fix this, use a current version of Node instead (the one
recommended in our setup instructions at the top of this page.) You
can check what version is installed by running the command
`node --version`.

[jest-babel-discussion]: https://github.com/zulip/zulip-mobile/pull/3619#issuecomment-533349362

Expand Down
3 changes: 3 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@
"test:flow-todo": "eslint --no-eslintrc -c tools/flow-todo.eslintrc.yaml src/",
"update-libdefs": "yarn install && rm -rf flow-typed/npm/ && flow-typed update"
},
"engines": {
"node": ">=10"
},
"dependencies": {
"@expo/react-native-action-sheet": "^3.8.0",
"@react-native-async-storage/async-storage": "^1.13.0",
Expand Down
17 changes: 0 additions & 17 deletions tools/test
Original file line number Diff line number Diff line change
Expand Up @@ -188,22 +188,6 @@ run_lint() {
eslint ${fix:+--fix} --max-warnings=0 "${files[@]}"
}

check_node() {
local node_version
node_version="$(node --version)"
case "${node_version}" in
v1[0-9].*) ;;
*)
cat >&2 <<EOF
Node 10 required; \`node --version\` says: ${node_version:-(nothing)}
To run the zulip-mobile tests, please install Node v10.x .
EOF
return 1 ;;
esac
}

run_jest() {
# Unlike some others, this inspects "$files" for itself.
local jest_args=()
Expand Down Expand Up @@ -236,7 +220,6 @@ run_jest() {
sloppy) jest_args+=( --selectProjects "${platforms[RANDOM%2]}" );;
esac

check_node || return
jest "${jest_args[@]}"
}

Expand Down

0 comments on commit 70cf310

Please sign in to comment.