Skip to content

chore(treewide): natively support CommonJS #334

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

Merged
nbsp merged 8 commits into from
Nov 28, 2024
Merged

chore(treewide): natively support CommonJS #334

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
613852f
chore: support CommonJS modules
Nov 13, 2024
35b881b
2
Nov 13, 2024
09afb76
camelcase-keys
Nov 18, 2024
b1b3136
Merge branch 'main' into nbsp/chore/cjs
Nov 27, 2024
8095937
update @livekit/protocol
Nov 27, 2024
7840443
don't bundle files
Nov 27, 2024
9d82e89
Create brave-terms-pretend.md
lukasIO Nov 28, 2024
976821c
Update brave-terms-pretend.md
Nov 28, 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
6 changes: 6 additions & 0 deletions .changeset/brave-terms-pretend.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"@livekit/rtc-node": minor
"livekit-server-sdk": minor
---

natively support CommonJS
2 changes: 1 addition & 1 deletion REUSE.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ SPDX-License-Identifier = "Apache-2.0"

# project configuration files
[[annotations]]
path = ["packages/livekit-rtc/.npmignore", ".prettierrc", ".prettierignore", ".eslintrc", "**.json", ".npmrc"]
path = ["packages/livekit-rtc/.npmignore", ".prettierrc", ".prettierignore", ".eslintrc", "**.json", ".npmrc", "**/tsup.config.ts"]
SPDX-FileCopyrightText = "2024 LiveKit, Inc."
SPDX-License-Identifier = "Apache-2.0"

Expand Down
2 changes: 1 addition & 1 deletion examples/agent-dispatch/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
"dependencies": {
"dotenv": "^16.4.5",
"livekit-server-sdk": "workspace:*",
"@livekit/protocol": "^1.27.1"
"@livekit/protocol": "^1.28.0"
},
"devDependencies": {
"@types/node": "^20.10.4",
Expand Down
15 changes: 12 additions & 3 deletions packages/livekit-rtc/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,15 @@
"author": "LiveKit",
"version": "0.11.1",
"main": "dist/index.js",
"require": "dist/index.cjs",
"types": "dist/index.d.ts",
"exports": {
".": {
"types": "./dist/index.d.ts",
"import": "./dist/index.js",
"require": "./dist/index.cjs"
}
},
"type": "module",
"repository": {
"type": "git",
Expand Down Expand Up @@ -42,6 +50,7 @@
"@napi-rs/cli": "^2.18.0",
"@types/node": "^20.9.2",
"prettier": "^3.0.3",
"tsup": "^8.3.5",
"typescript": "^5.2.2"
},
"optionalDependencies": {
Expand All @@ -56,9 +65,9 @@
},
"scripts": {
"prebuild": "node -p \"'export const SDK_VERSION = ' + JSON.stringify(require('./package.json').version) + ';'\" > src/version.ts",
"build:tsc": "pnpm prebuild && tsc && cp -r src/napi dist/",
"build": "pnpm build:tsc && napi build --platform --release --dts native.d.ts --js native.cjs --pipe \"prettier -w\" src/napi",
"artifacts": "pnpm build:tsc && napi artifacts",
"build:ts": "pnpm prebuild && tsup --onSuccess \"tsc --declaration --emitDeclarationOnly\" && cp -r src/napi dist/ && cp -r src/napi/* dist/",
Copy link
Copy Markdown
Author

@nbsp nbsp Nov 13, 2024

Choose a reason for hiding this comment

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

this line is silly but it's to get around esbuild's issues with dynamic import of fs

Copy link
Copy Markdown
Contributor

@lukasIO lukasIO Nov 27, 2024

Choose a reason for hiding this comment

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

is the order of things correct here? it looks like it first copies napi files but napi build is only called afterwards in the build script 👀

Copy link
Copy Markdown
Contributor

@lukasIO lukasIO Nov 27, 2024

Choose a reason for hiding this comment

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

hm it was like this also before, just not sure I understand the reason 🤷

Copy link
Copy Markdown
Author

@nbsp nbsp Nov 27, 2024

Choose a reason for hiding this comment

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

there's a few files in src/napi already pre-generation, namely the shim to translate them from CJS to ESM if needed. i do think this might be a bit more convoluted than necessary but it works and doesn't seem to take too long on this part

"build": "pnpm build:ts && napi build --platform --release --dts native.d.ts --js native.cjs --pipe \"prettier -w\" src/napi",
Copy link
Copy Markdown
Contributor

@lukasIO lukasIO Nov 13, 2024

Choose a reason for hiding this comment

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

this looks like napi is only building for cjs, is that the behaviour we want?

Copy link
Copy Markdown
Author

@nbsp nbsp Nov 13, 2024

Choose a reason for hiding this comment

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

we have a wrapper for esm, napi only generates CJS

Copy link
Copy Markdown
Author

@nbsp nbsp Nov 21, 2024

Choose a reason for hiding this comment

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

update: this doesn't actually work, tried using agents and it broke. unsure where to go from here because napi doesn't do ESM

Copy link
Copy Markdown
Contributor

@lukasIO lukasIO Nov 21, 2024

Choose a reason for hiding this comment

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

what's the error you're running into?

Copy link
Copy Markdown
Author

@nbsp nbsp Nov 21, 2024

Choose a reason for hiding this comment

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

something about not being able to find the native.cjs file when running it under cjs. can't reproduce right now, i'm having a different issue related to tsup on Agents in specific

Copy link
Copy Markdown
Author

@nbsp nbsp Nov 21, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

upon further testing with node-sdks examples directly this doesn't appear to be reproducible 🤷‍♀️

"artifacts": "pnpm build:ts && napi artifacts",
"build:debug": "napi build --platform",
"lint": "eslint -f unix \"src/**/*.ts\" --ignore-pattern \"src/proto/*\"",
"universal": "napi universal",
Expand Down
4 changes: 2 additions & 2 deletions packages/livekit-rtc/src/audio_source.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ export class AudioSource {

get queuedDuration(): number {
return Math.max(
this.currentQueueSize - Number(process.hrtime.bigint() / 1000000n) + this.lastCapture,
this.currentQueueSize - Number(process.hrtime.bigint() / BigInt(1000000)) + this.lastCapture,
0,
);
}
Expand Down Expand Up @@ -102,7 +102,7 @@ export class AudioSource {
}

async captureFrame(frame: AudioFrame) {
const now = Number(process.hrtime.bigint() / 1000000n);
const now = Number(process.hrtime.bigint() / BigInt(1000000));
const elapsed = this.lastCapture === 0 ? 0 : now - this.lastCapture;
const frameDurationMs = (frame.samplesPerChannel / frame.sampleRate) * 1000;
this.currentQueueSize += frameDurationMs - elapsed;
Expand Down
4 changes: 2 additions & 2 deletions packages/livekit-rtc/src/e2ee.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ export class KeyProvider {
}

export class FrameCryptor {
private roomHandle = 0n;
private roomHandle = BigInt(0);
participantIdentity: string;
keyIndex: number;
enabled: boolean;
Expand Down Expand Up @@ -246,7 +246,7 @@ export class FrameCryptor {
}

export class E2EEManager {
private roomHandle = 0n;
private roomHandle = BigInt(0);
private options: E2EEOptions;
private keyProvider?: KeyProvider;

Expand Down
10 changes: 5 additions & 5 deletions packages/livekit-rtc/src/napi/native.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,15 @@

/* auto-generated by NAPI-RS */

export declare function livekitInitialize(
export function livekitInitialize(
callback: (data: Uint8Array) => void,
captureLogs: boolean,
sdkVersion: string,
): void;
export declare function livekitFfiRequest(data: Uint8Array): Uint8Array;
export declare function livekitRetrievePtr(handle: Uint8Array): bigint;
export declare function livekitCopyBuffer(ptr: bigint, len: number): Uint8Array;
export declare function livekitDispose(): Promise<void>;
export function livekitFfiRequest(data: Uint8Array): Uint8Array;
export function livekitRetrievePtr(handle: Uint8Array): bigint;
export function livekitCopyBuffer(ptr: bigint, len: number): Uint8Array;
export function livekitDispose(): Promise<void>;
export declare class FfiHandle {
constructor(handle: bigint);
dispose(): void;
Expand Down
6 changes: 5 additions & 1 deletion packages/livekit-rtc/src/video_source.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,11 @@ export class VideoSource {
this.ffiHandle = new FfiHandle(res.source!.handle!.id);
}

captureFrame(frame: VideoFrame, timestampUs = 0n, rotation = VideoRotation.VIDEO_ROTATION_0) {
captureFrame(
frame: VideoFrame,
timestampUs = BigInt(0),
rotation = VideoRotation.VIDEO_ROTATION_0,
) {
const req = create(CaptureVideoFrameRequestSchema, {
sourceHandle: this.ffiHandle.handle,
buffer: frame.protoInfo(),
Expand Down
6 changes: 3 additions & 3 deletions packages/livekit-rtc/tsconfig.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
{
"compilerOptions": {
"module": "ES2020",
"module": "ES2015",
Copy link
Copy Markdown
Contributor

@lukasIO lukasIO Nov 13, 2024

Choose a reason for hiding this comment

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

what's the oldest node version we want to support?

Copy link
Copy Markdown
Author

@nbsp nbsp Nov 13, 2024

Choose a reason for hiding this comment

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

unsure but if it works with es2015 it'll work with all of them. before es2015 there was no const/let, so it's not worth going back any more

"esModuleInterop": true,
"allowJs": true,
"declaration": true,
"declarationMap": true,
"target": "es2020",
"lib": ["es2020"],
"target": "es2015",
"lib": ["es2015"],
"moduleResolution": "node",
"sourceMap": true,
"outDir": "dist",
Expand Down
8 changes: 8 additions & 0 deletions packages/livekit-rtc/tsup.config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import { defineConfig } from 'tsup';

import defaults from '../../tsup.config';

export default defineConfig({
...defaults,
external: [/\.\/.*\.cjs/, /\.\/.*.node/],
});
13 changes: 11 additions & 2 deletions packages/livekit-server-sdk/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,25 @@
"version": "2.8.1",
"description": "Server-side SDK for LiveKit",
"main": "dist/index.js",
"require": "dist/index.cjs",
"types": "dist/index.d.ts",
"repository": "git@github.com:livekit/server-sdk-js.git",
"author": "David Zhao <david@davidzhao.com>",
"license": "Apache-2.0",
"type": "module",
"exports": {
".": {
"types": "./dist/index.d.ts",
"import": "./dist/index.js",
"require": "./dist/index.cjs"
}
},
"files": [
"dist",
"src"
],
"scripts": {
"build": "tsc --project tsconfig.json",
"build": "tsup --onSuccess \"tsc --declaration --emitDeclarationOnly\"",
"build:watch": "tsc --watch",
"build-docs": "typedoc",
"changeset": "changeset",
Expand All @@ -26,7 +34,7 @@
"test:edge": "vitest --environment edge-runtime run"
},
"dependencies": {
"@livekit/protocol": "^1.27.1",
"@livekit/protocol": "^1.28.1",
"camelcase-keys": "^9.0.0",
"jose": "^5.1.2"
},
Expand All @@ -38,6 +46,7 @@
"@types/node": "^20.10.1",
"happy-dom": "^15.0.0",
"prettier": "^3.0.0",
"tsup": "^8.3.5",
"typedoc": "^0.26.0",
"typescript": "5.6.x",
"vite": "^5.2.9",
Expand Down
2 changes: 1 addition & 1 deletion packages/livekit-server-sdk/src/TwirpRPC.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
//
// SPDX-License-Identifier: Apache-2.0
import type { JsonValue } from '@bufbuild/protobuf';
import camelcaseKeys from 'camelcase-keys';

// twirp RPC adapter for client implementation

Expand Down Expand Up @@ -51,6 +50,7 @@ export class TwirpRpc {
throw new Error(`Request failed with status ${response.status}: ${response.statusText}`);
}
const parsedResp = await response.json();
const camelcaseKeys = await import('camelcase-keys').then((mod) => mod.default);
return camelcaseKeys(parsedResp, { deep: true });
}
}
7 changes: 7 additions & 0 deletions packages/livekit-server-sdk/tsup.config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import { defineConfig } from 'tsup';

import defaults from '../../tsup.config';

export default defineConfig({
...defaults,
});
Loading