Skip to content
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.

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

Node 18 support #206

Merged
merged 7 commits into from
Aug 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
8 changes: 6 additions & 2 deletions .github/workflows/run-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,10 @@ jobs:
# TURBO_TEAM: ${{ vars.TURBO_TEAM }}
# TURBO_REMOTE_ONLY: true

strategy:
matrix:
node-version: ['18.x', '20.x', '22.x']

steps:
- name: Check out code
uses: actions/checkout@v4
Expand All @@ -27,10 +31,10 @@ jobs:
with:
version: 9

- name: Setup Node.js environment
- name: Setup Node.js ${{ matrix.node-version }}
uses: actions/setup-node@v4
with:
node-version: 22
node-version: ${{ matrix.node-version }}
cache: 'pnpm'

- name: Install dependencies
Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ Srcbook runs locally on your machine as a CLI application with a web interface.

### Requirements

- Node 20+, we recommend using [nvm](https://github.com/nvm-sh/nvm) to manage local node versions
- Node 18+, we recommend using [nvm](https://github.com/nvm-sh/nvm) to manage local node versions
- [corepack](https://nodejs.org/api/corepack.html) to manage package manager versions

### Installing
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,6 @@
},
"packageManager": "[email protected]",
"engines": {
"node": ">=20"
"node": ">=18"
}
}
9 changes: 8 additions & 1 deletion packages/shared/src/utils.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,15 @@
import { base32hexnopad } from '@scure/base';
import type { CodeLanguageType } from './types/cells.js';
import * as crypto from 'crypto';
Copy link
Contributor

Choose a reason for hiding this comment

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

This is used on the client in addition to the Node backend. What happens in that environment? Is vite smart enough to remove this from the compiled code, or does it try to do some sort of polyfill (e.g. the way webpack used to with crypto-browserify)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This should technically work Vite doesn't do a polyfil but ignores the import which should then cause it to end up using the global window crypto instance, or polyfills can be added if for some reason it's not but it does seem to be working

Copy link
Contributor

Choose a reason for hiding this comment

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

I do not want us to use polyfills for something that exists natively. So assuming this drops the import then I'm stoked to get this working in node 18.


export function isBrowser(): boolean {
return typeof window !== 'undefined';
}

export function randomid(byteSize = 16) {
const bytes = crypto.getRandomValues(new Uint8Array(byteSize));
const bytes = isBrowser()
? globalThis.crypto.getRandomValues(new Uint8Array(byteSize))
: crypto.getRandomValues(new Uint8Array(byteSize));
return base32hexnopad.encode(bytes).toLowerCase();
}

Expand Down
2 changes: 1 addition & 1 deletion srcbook/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ Srcbook runs locally on your machine as a CLI application with a web interface.

### Requirements

- Node.js v20+
- Node.js v18+
versecafe marked this conversation as resolved.
Show resolved Hide resolved
- We recommend using [nvm](https://github.com/nvm-sh/nvm) to manage local node versions

### Installing
Expand Down
2 changes: 1 addition & 1 deletion srcbook/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,6 @@
"@types/express": "^4.17.21"
},
"engines": {
"node": ">=20"
"node": ">=18"
}
}