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

feat: add types support to esm bundle for lightclient #6685

Merged
merged 7 commits into from
Apr 30, 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
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@
"typescript-docs-verifier": "^2.5.0",
"vite-plugin-node-polyfills": "^0.21.0",
"vite-plugin-top-level-await": "^1.4.1",
"vite-plugin-dts": "^3.8.3",
"vite": "^5.2.9",
"vitest": "^1.5.0",
"vitest-when": "^0.3.1",
Expand Down
3 changes: 2 additions & 1 deletion packages/api/src/beacon/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@ import type {Api} from "./routes/index.js";

// NOTE: Don't export server here so it's not bundled to all consumers

export * as routes from "./routes/index.js";
import * as routes from "./routes/index.js";
export {routes};
export {getClient} from "./client/index.js";
export type {Api};

Expand Down
6 changes: 1 addition & 5 deletions packages/api/src/beacon/routes/beacon/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,7 @@ import * as rewards from "./rewards.js";

// NOTE: We choose to split the block, pool, and state namespaces so the files are not too big.
// However, for a consumer all these methods are within the same service "beacon"

export * as block from "./block.js";
export * as pool from "./pool.js";
export * as state from "./state.js";
export * as rewards from "./rewards.js";
export {block, pool, state, rewards};
export {BroadcastValidation} from "./block.js";
export type {BlockId, BlockHeaderResponse} from "./block.js";
export type {AttestationFilters} from "./pool.js";
Expand Down
19 changes: 10 additions & 9 deletions packages/api/src/beacon/routes/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,16 @@ import {Api as NodeApi} from "./node.js";
import {Api as ProofApi} from "./proof.js";
import {Api as ValidatorApi} from "./validator.js";

export * as beacon from "./beacon/index.js";
export * as config from "./config.js";
export * as debug from "./debug.js";
export * as events from "./events.js";
export * as lightclient from "./lightclient.js";
export * as lodestar from "./lodestar.js";
export * as node from "./node.js";
export * as proof from "./proof.js";
export * as validator from "./validator.js";
import * as beacon from "./beacon/index.js";
import * as config from "./config.js";
import * as debug from "./debug.js";
import * as events from "./events.js";
import * as lightclient from "./lightclient.js";
import * as lodestar from "./lodestar.js";
import * as node from "./node.js";
import * as proof from "./proof.js";
import * as validator from "./validator.js";
export {beacon, config, debug, events, lightclient, lodestar, node, proof, validator};

export type Api = {
beacon: BeaconApi;
Expand Down
4 changes: 4 additions & 0 deletions packages/light-client/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,10 @@ NOTE: Due to `top-level-await` used in one of dependent library, the package wil
}, 50);
});
</script>

**Typescript support** The web bundle comes with the types support. Unfortunately due to following
[issue](https://github.com/microsoft/rushstack/issues/1128#issuecomment-2066257538) we can't bundle all types. A
workaround would be to add `@chainsafe/as-sha256` as a devDependency to your project.
```

## Contributors
Expand Down
2 changes: 1 addition & 1 deletion packages/light-client/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@
"build:watch": "yarn run build --watch",
"build:release": "yarn clean && yarn run build && yarn run build:dist",
"check-build": "node -e \"(async function() { await import('./lib/index.js') })()\"",
"check-types": "tsc",
"check-types": "yarn run build:dist && tsc",
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we need this?

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 checks the tests file as well and one test file is using the types imported from the bundle. So we have to build it before running types check.

Copy link
Member

Choose a reason for hiding this comment

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

Can we solve this differently? Having this build run as part of check-types is really annoying during local development. check-types is supposed to be fast and not create a bundle for production

This takes more time than all other check-types across all packages combined

@lodestar/light-client: vite v5.2.10 building for production...
@lodestar/light-client: transforming...
@lodestar/light-client: ✓ 562 modules transformed.
@lodestar/light-client: rendering chunks...
@lodestar/light-client: [vite:dts] Start generate declaration files...
@lodestar/light-client: computing gzip size...
@lodestar/light-client: [vite:dts] Start rollup declaration files...
@lodestar/light-client: Analysis will use the bundled TypeScript version 5.4.2
@lodestar/light-client: [vite:dts] Declaration files built in 6702ms.
@lodestar/light-client: dist/manifest.json            0.13 kB │ gzip:   0.11 kB
@lodestar/light-client: dist/lightclient.min.mjs  1,160.16 kB │ gzip: 341.51 kB │ map: 2,887.35 kB
@lodestar/light-client: ✓ built in 12.17s

It also creates a untracked file

Untracked files:
  (use "git add <file>..." to include in what will be committed)
        packages/light-client/stats.html

Copy link
Member

Choose a reason for hiding this comment

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

it also randomly fails, see CI https://github.com/ChainSafe/lodestar/actions/runs/8911114728/job/24471848781#step:4:124

@lodestar/light-client: test/unit/webEsmBundle.browser.test.ts:3:63 - error TS2307: Cannot find module '../../dist/lightclient.min.mjs' or its corresponding type declarations.
@lodestar/light-client: 3 import {Lightclient, LightclientEvent, utils, transport} from "../../dist/lightclient.min.mjs";

"lint": "eslint --color --ext .ts src/ test/",
"lint:fix": "yarn run lint --fix",
"test": "yarn test:unit",
Expand Down
7 changes: 4 additions & 3 deletions packages/light-client/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -320,6 +320,7 @@ export class Lightclient {
}

// To export these name spaces to the bundle JS
export * as utils from "./utils.js";
export * as validation from "./validation.js";
export * as transport from "./transport.js";
import * as utils from "./utils.js";
import * as validation from "./validation.js";
import * as transport from "./transport.js";
export {utils, validation, transport};
15 changes: 4 additions & 11 deletions packages/light-client/test/unit/webEsmBundle.browser.test.ts
Original file line number Diff line number Diff line change
@@ -1,21 +1,16 @@
/* eslint-disable @typescript-eslint/no-unsafe-assignment,@typescript-eslint/no-unsafe-member-access,@typescript-eslint/no-unsafe-call */
import {expect, describe, it, beforeEach, vi} from "vitest";
import "../../dist/lightclient.min.mjs";
import {expect, describe, it, vi} from "vitest";
import {Lightclient, LightclientEvent, utils, transport} from "../../dist/lightclient.min.mjs";

describe("web bundle for lightclient", () => {
vi.setConfig({testTimeout: 20_000});

let lightclient: any;

beforeEach(() => {
lightclient = (window as any)["lodestar"]["lightclient"];
});

it("should have a global interface", () => {
expect(lightclient).toBeDefined();
expect((window as any)["lodestar"]["lightclient"]).toBeDefined();
});

it("should have all relevant exports", () => {
const lightclient = (window as any)["lodestar"]["lightclient"];
expect(lightclient).toHaveProperty("Lightclient");
expect(lightclient).toHaveProperty("LightclientEvent");
expect(lightclient).toHaveProperty("RunStatusCode");
Expand All @@ -29,8 +24,6 @@ describe("web bundle for lightclient", () => {
});

it("should start the lightclient and sync", async () => {
const {Lightclient, LightclientEvent, transport, utils} = lightclient;

const logger = utils.getConsoleLogger({logDebug: true});
const config = utils.getChainForkConfigFromNetwork("mainnet");

Expand Down
15 changes: 15 additions & 0 deletions packages/light-client/vite.config.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,26 @@
import path from "node:path";
import fs from "node:fs/promises";
import {defineConfig, mergeConfig} from "vite";
import dts from "vite-plugin-dts";
import {getBaseViteConfig} from "../../vite.base.config.js";

import pkgJSON from "./package.json";

export default mergeConfig(
getBaseViteConfig(pkgJSON, {libName: "LightClient", entry: "src/index.ts"}),
defineConfig({
plugins: [
dts({
rollupTypes: true,
bundledPackages: ["@lodestar/*", "@chainsafe/persistent-merkle-tree", "@chainsafe/bls", "@chainsafe/ssz"],
Copy link
Contributor

Choose a reason for hiding this comment

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

How do you come up with this list? Is there no way to automate this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

By building the types buyndle and investigating the types import in it. The ideal scenario we want is there should not be any thirdparty types import in types bundle.

async afterBuild() {
await fs.rename(
path.join(import.meta.dirname, "dist", "index.d.ts"),
path.join(import.meta.dirname, "dist", "lightclient.min.d.mts")
);
},
}),
],
build: {
rollupOptions: {
output: {
Expand Down
8 changes: 6 additions & 2 deletions packages/types/src/allForks/index.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
export * from "./types.js";
export * as ts from "./types.js";
export * as ssz from "./sszTypes.js";

// We have to use import->export because of the limitation in `@microsoft/api-extractor`
// which is used to bundle the package types
import * as ts from "./types.js";
import * as ssz from "./sszTypes.js";
export {ts, ssz};
5 changes: 3 additions & 2 deletions packages/types/src/altair/index.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
export * from "./types.js";
export * as ts from "./types.js";
export * as ssz from "./sszTypes.js";
import * as ts from "./types.js";
import * as ssz from "./sszTypes.js";
export {ts, ssz};
5 changes: 3 additions & 2 deletions packages/types/src/bellatrix/index.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
export * from "./types.js";
export * as ts from "./types.js";
export * as ssz from "./sszTypes.js";
import * as ts from "./types.js";
import * as ssz from "./sszTypes.js";
export {ts, ssz};
5 changes: 3 additions & 2 deletions packages/types/src/capella/index.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
export * from "./types.js";
export * as ts from "./types.js";
export * as ssz from "./sszTypes.js";
import * as ts from "./types.js";
import * as ssz from "./sszTypes.js";
export {ts, ssz};
5 changes: 3 additions & 2 deletions packages/types/src/deneb/index.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
export * from "./types.js";
export * as ts from "./types.js";
export * as ssz from "./sszTypes.js";
import * as ts from "./types.js";
import * as ssz from "./sszTypes.js";
export {ts, ssz};
3 changes: 2 additions & 1 deletion packages/types/src/index.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
export * from "./types.js";
export * as ssz from "./sszTypes.js";
import * as ssz from "./sszTypes.js";
export {ssz};
// Typeguards
export * from "./utils/typeguards.js";
// String type
Expand Down
5 changes: 3 additions & 2 deletions packages/types/src/phase0/index.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
export * from "./types.js";
export * as ts from "./types.js";
export * as ssz from "./sszTypes.js";
import * as ts from "./types.js";
import * as ssz from "./sszTypes.js";
export {ts, ssz};
5 changes: 3 additions & 2 deletions packages/types/src/primitive/index.ts
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
export * as ts from "./types.js";
export * as ssz from "./sszTypes.js";
import * as ts from "./types.js";
import * as ssz from "./sszTypes.js";
export {ts, ssz};
Loading
Loading