Repository navigation
test: Add package build configuration tests - #2116
Conversation
Co-Authored-By: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com>
Co-Authored-By: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com>
…-native, cloudflare and jest packages Co-Authored-By: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com>
Co-Authored-By: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com>
|
I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".
|
ts-jest 29.4.15 resolves test imports with the tsconfig's moduleResolution (nodenext), so the extensionless '../src/index' import no longer resolves. Co-Authored-By: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com>
a9b4178 to
45fbfc5
Compare
Co-Authored-By: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com>
Co-Authored-By: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com>
Co-Authored-By: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com>
Co-Authored-By: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com>
…minimum-age gate Co-Authored-By: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com>
45fbfc5 to
6961e68
Compare
| paths-ignore: | ||
| - '**.md' | ||
| pull_request: | ||
| branches: [main, 'feat/**', 'devin/1792686117-node16-types'] |
There was a problem hiding this comment.
nit: should remove the temporary branch once the fix is merged
There was a problem hiding this comment.
Will wait until first PR merged before approving this one.
Co-Authored-By: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com>
Fixes TypeScript type resolution under `moduleResolution: node16`/`nodenext` for `@launchdarkly/node-client-sdk`, its common packages, and the sibling packages with the same packaging problems. **Related issues** Fixes #2113 - `node-client-sdk`, `electron-client-sdk`, `react-sdk`, `vue-client-sdk`, `client-testing-plugin`, `cloudflare-server-sdk`, `openfeature-node-server`, `openfeature-cloudflare-server`: `exports` now gives each condition its own declarations (`import` → ESM `.d.ts`, `require` → `.d.cts` / CJS `.d.ts`). Before, CJS consumers were type-checked against the ESM declarations (TS1479, "Masquerading as ESM"). - `js-sdk-common`, `js-client-sdk-common`, `js-server-sdk-common-edge`, `vercel-server-sdk`, `react-native-client-sdk`, `jest`: a new post-build step, `scripts/add-esm-declaration-extensions.js`, adds explicit `.js` / `/index.js` to relative imports in emitted ESM `.d.ts` files (TS2834, "Internal resolution error"). JS output is unchanged. - `@launchdarkly/jest`: removed the `./react` export. `src/react` doesn't exist, so it never resolved (types or runtime) in any published version. - Main-vs-PR regression matrix over every released package: 0 regressions (details below). **Requirements** - [ ] I have added test coverage for new or changed functionality - [x] I have followed the repository's [pull request submission guidelines](../blob/main/CONTRIBUTING.md#submitting-pull-requests) - [x] I have validated my changes against all supported platform versions <details> <summary>Implementation details</summary> **Describe the solution you've provided** Per-package changes beyond the two mechanisms above: - `cloudflare-server-sdk`: the CJS rollup build now also emits declarations into `dist/cjs`, `exports` maps `require` to them, and the build writes `dist/cjs/package.json` with `"type": "commonjs"`. Before, the package is `"type": "module"`, so Node treated `dist/cjs/index.js` as ESM: `require('@launchdarkly/cloudflare-server-sdk')` returned no exports on Node 24. It now returns the real exports. - `openfeature-node-server` / `openfeature-cloudflare-server`: switched from a separate `tsc --emitDeclarationOnly` (one ESM-flavoured `.d.ts` for both conditions) to tsup `dts: true`, which emits `index.d.ts` + `index.d.cts`. - `js-server-sdk-common-edge` / `vercel-server-sdk`: the rewrite runs inside `scripts/build-package.sh` on `dist/esm`. **Regression matrix (main `578bdebbf2` vs this branch)** Every package in `.release-please-manifest.json` was `yarn pack`ed from both trees and installed into a fresh npm consumer project. - `tsc` consumer per package entrypoint (`import * as m from '<pkg>'`) × `node10`, `node16` CJS/ESM, `nodenext` CJS/ESM, `bundler`, `module: preserve` × TypeScript 5.0.4, 5.9.3 and 6.0.3, with `strict` and `skipLibCheck: false` (700 cases): 0 regressions, 198 improved, the rest unchanged. Every package except `react-native-client-sdk` and `jest` is now 0 errors in all node16/nodenext/bundler modes on TS 5.9/6.0. Errors left over are the same on main: TS 6's node10 deprecation (TS5107), `fastly:kv-store` types, and the test project not having `@types/jest`. - `@arethetypeswrong/cli` (node10, node16 CJS/ESM, bundler): no new problems on any package. 17 problems fixed. Remaining: node10 can't read subpath `exports` (`react-sdk`, `client-testing-plugin`, `jest`, same as before), and `react-native-client-sdk` / `jest/react-native` CJS → ESM. Those packages only ship ESM, so TS1479 for a node16 CJS consumer is accurate. - Runtime, Node 24: `require` and `import` of every entrypoint, plus esbuild bundles (esm/cjs × node/browser/neutral). Results match main everywhere except `cloudflare-server-sdk` `require`, which is now fixed (see above). - Root `yarn build` passes. Unit tests, lint and `oxfmt --check` pass for every changed workspace. The original repro from the issue (TS 5.9.3): published 4.3.0 gives 68–69 errors under node16/nodenext for both `index.ts` and `index.mts`. With this PR it gives 0. **Describe alternatives you've considered** - **Bundling the ESM declarations** (tsup `--dts-only`): breaks declaration emit downstream. `js-client-sdk-common` can't name internal `js-sdk-common` types such as `DiagnosticInitEvent` (TS4061/TS4094). - **Adding `.js` extensions in source**: about 570 relative imports in the common packages alone, plus Jest `moduleNameMapper` changes and a lint rule. - **Moving the common packages to tsup**: changes `dist` layout, size-check paths and JS output for every SDK. **Edge runtime validation** (packed `main` vs PR builds): - Cloudflare: Workers for `cloudflare-server-sdk` (TS, tsup prebuild and a CommonJS `require` entry) and `openfeature-cloudflare-server`, run with `wrangler dev` (workerd), wrangler 4.152.0 and 3.114.17, `nodejs_compat`, flag data in local KV. Each Worker initialized and returned the expected `FALLTHROUGH` / `true` evaluation. The ESM `wrangler deploy --dry-run` bundles are byte-identical between `main` and the PR. The `require` Worker fails to start on `main` (`ReferenceError: exports is not defined` in `dist/cjs/index.js`) and works on the PR. - Vercel: Next.js 16.4.2 app with an edge route handler and edge middleware using `@vercel/edge-config` 1.5.1, pointed at a local Edge Config mock. `next build` (Turbopack and `--webpack`) plus `next start`: both routes evaluate correctly on `main` and the PR. **Additional context** The rewrite script fails the build if a relative specifier doesn't resolve to a `.d.ts` file or an `index.d.ts` directory, so a broken rewrite can't ship silently. `sdk/akamai-base` unit tests started failing on `main` with `TS2307: Cannot find module '../src/index'` after ts-jest 29.4.15 (released 2026-10-15), which now resolves test imports with the tsconfig's `moduleResolution: nodenext`. `jest.config.json` now tells ts-jest to use `moduleResolution: node` for tests; the build config is unchanged. `cloudflare-server-sdk` now declares `semver` (7.5.4, the same as `js-server-sdk-common`). Its rollup bundle inlines the js-core packages but leaves `semver` external, so `dist/*/index.js` requires `semver/functions/parse.js`. The package didn't declare it, so whether the require resolved depended on the package manager hoisting another package's semver 7. The Yarn-based package-config tests in #2116 caught it failing to load. Contract-test steps intermittently fail with `Unable to find a release matching 'v2'` / `'v3'`. That comes from the sdk-test-harness downloader's release lookup, not from the SDK; the latest run passed every contract-test job. </details> Link to Devin session: https://app.devin.ai/sessions/6455af0ee31c4e5fbfd20c108731cfa7 Open in Devin Desktop: https://app.devin.ai/desktop/session/6455af0ee31c4e5fbfd20c108731cfa7?variant=devin Requested by: @kinyoklion --------- Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…anch" This reverts commit c025e7a.
|
@launchdarkly/js-sdk-common size report |
|
@launchdarkly/js-client-sdk size report |
|
@launchdarkly/js-client-sdk-common size report |
Adds a CI workflow that packs every released package and tests the tarballs in fresh Yarn consumer projects, under many build configurations and in the Cloudflare and Vercel runtimes. Results are compared against a checked-in baseline. Follows #2115.
package-configs: one job builds and packs all released packages, then 7 parallel jobs run the suites against the tarballs. Runs on PRs and pushes tomain/feat/**(markdown-only changes skipped), nightly, and on manual dispatch.types(tsc for every entrypoint under node10, node16/nodenext CJS and ESM, bundler, preserve; TS 5.0.4, 5.9.3, 6.0.3;strict,skipLibCheck: false),attw,runtime(Noderequire/import, esbuild node/browser/neutral),cloudflare(workerd throughwrangler devwith local KV),vercel(Next.js edge route and middleware, Turbopack and webpack).scripts/package-config-tests/baseline/*.jsonfails the job. A case that wasokand now fails is reported as a regression. An intended change needs--updateand a committed baseline diff.Implementation details
Requirements
Related issues
#2113, #2115.
Describe the solution you've provided
scripts/package-config-tests/run.js <suite>: runs one suite (pack,types[:<ts version>],attw,runtime,cloudflare,vercel) and compares the results withbaseline/<suite>.json. Use--updateto accept new results.<work>/<suite>-details.jsonhas the error output for each failing case.resolutionsentry, so packages that depend on each other resolve to the local builds. Direct tool versions (TypeScript,@types/node, React, esbuild, ATTW, wrangler, Next.js,@vercel/edge-config, ...) are pinned so that results stay stable..yarnrc.ymlbuilt fromfixtures/consumer.yarnrc.yml(static settings) plusnpmMinimalAgeGate(3 days) andnpmPreapprovedPackagesfrom the repository's Yarn config, so external dependencies, transitive ones included, go through the same quarantine as the monorepo's own installs. The LaunchDarkly tarballs are local files and are not age-gated. The@bytecodealliance/jcoTypeScript resolution is copied from the rootpackage.json. Yarn doesn't install peer dependencies, so the installer adds any missing required peers and reinstalls, as npm would.okor a short failure value: TypeScript error codes, ATTW problem kinds, orfail. Full messages go only into the details file, so unrelated wording changes don't fail the job.wrangler dev, and must returnvalue: trueforsample-feature. Each Worker also runswrangler deploy --dry-run. The four Workers are: a TypeScript Worker, the OpenFeature provider, a Worker using CommonJSrequire, and one prebuilt with tsup as inpackages/sdk/cloudflare/example.next start. The edge route and the middleware must both evaluate the flag. The real@vercel/edge-configclient reads from a local mock of the Edge Config item API, since CI has no Vercel store.okvalues. Examples: node10 has no subpath support,react-native-client-sdk/jest/react-nativeare ESM-only (TS1479 for node16 CJS), Node-only packages fail browser/neutral bundles, edge SDKs don't load in plain Node, andfastly-server-sdkneedsfastly:*types.Validation
main: the suites fail as they should. The Cloudflarecjs-requireWorker is reported as a regression (it doesn't start onmain).attwandruntimereport the differences that fix: Resolve types under node16/nodenext module resolution #2115 fixed.types:*5.5–8 min each, the other suites under 1 min each.Describe alternatives you've considered
Additional context
Found while running the suite:
server-sdk-ai-langchaingives TS1479 for node16 CommonJS consumers. It is recorded in the baseline as an existing limitation.Also found after moving to Yarn:
cloudflare-server-sdk's bundle requiressemver/functions/parse.jswithout declaringsemver. It only loaded when another package's semver 7 got hoisted. Fixed in #2115.Link to Devin session: https://app.devin.ai/sessions/6455af0ee31c4e5fbfd20c108731cfa7
Open in Devin Desktop: https://app.devin.ai/desktop/session/6455af0ee31c4e5fbfd20c108731cfa7?variant=devin
Requested by: @kinyoklion