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.

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

Jump to bottom

Node 18 support #206

Merged
merged 7 commits into from
Aug 25, 2024
Merged

Node 18 support #206

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
6aa0199
provide support for Node 18 LTS
versecafe Aug 23, 2024
965a465
update min node docs
versecafe Aug 23, 2024
28f78be
Merge branch 'main' into node-18-support
versecafe Aug 23, 2024
369f841
Merge branch 'srcbookdev:main' into node-18-support
versecafe Aug 25, 2024
7543ffd
fixup the final issues with the crypto fallbacks and warnings
versecafe Aug 25, 2024
1d1ae94
change readme min node & add to CI
versecafe Aug 25, 2024
f943a90
update engines rules
versecafe Aug 25, 2024
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
1 change: 1 addition & 0 deletions packages/shared/src/utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
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 randomid(byteSize = 16) {
const bytes = crypto.getRandomValues(new Uint8Array(byteSize));
Expand Down
2 changes: 1 addition & 1 deletion srcbook/README.md
View file
Open in desktop
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