Skip to content

refactor: simplify libsodium import#38

Merged
matejvasek merged 1 commit into
functions-dev:masterfrom
matejvasek:fixup-crypto-types
Jun 15, 2026
Merged

refactor: simplify libsodium import#38
matejvasek merged 1 commit into
functions-dev:masterfrom
matejvasek:fixup-crypto-types

Conversation

@matejvasek

@matejvasek matejvasek commented Jun 12, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Replace inline dynamic import('libsodium-wrappers') in GithubService.#encryptForGithub with a top-level default import
  • Remove SodiumFull type alias, double type cast, and runtime typeof guard
  • Add allowSyntheticDefaultImports to tsconfig for CJS default import compatibility
  • Remove deprecated @types/libsodium-wrappers stub (library ships its own types)

Detail

The #encryptForGithub method used a dynamic import with a custom SodiumFull intersection type and a (mod as unknown as { default?: SodiumFull }).default ?? mod fallback to work around webpack freezing the ESM namespace object (which prevented crypto functions from being populated after WASM init). Importing the default export directly avoids the immutable namespace entirely, so the workaround is no longer needed.

The method body goes from 17 lines to 5.

libsodium is now bundled eagerly instead of lazy-loaded, adding ~100 KiB to the main vendor chunk. Total bundle size is unchanged.

Test plan

  • yarn ci passes (lint, 126 tests, webpack build)
  • No new webpack warnings (the previous crypto_box_seal not found warning from import * is gone)
  • Manual: create a function repo and verify the KUBECONFIG secret is encrypted and set correctly

🤖 Generated with Claude Code

The #encryptForGithub method used an inline dynamic import
with a SodiumFull type downcast and default-export fallback
to work around webpack's immutable ESM namespace. The library
(0.8.x) provides its own types that cover crypto_box_seal as
a named export, so the workaround is unnecessary.

Replace with a top-level default import and await sodium.ready
at the call site. Add allowSyntheticDefaultImports to tsconfig
so TypeScript accepts the default import from the CJS types.
Remove the deprecated @types/libsodium-wrappers stub (the
library ships its own type definitions).

Note: libsodium is now bundled eagerly instead of lazy-loaded,
adding ~100 KiB to the main vendor chunk. Total bundle size
is unchanged.

Signed-off-by: Matej Vašek <matejvasek@gmail.com>
Co-Authored-By: Claude <noreply@anthropic.com>
@matejvasek matejvasek requested a review from twoGiants June 12, 2026 17:04
@matejvasek

Copy link
Copy Markdown
Collaborator Author

Two options for typing import sodium from 'libsodium-wrappers'

The problem: libsodium-wrappers ships two sets of type declarations. The CJS types (used by moduleResolution: "node") declare crypto functions as named exports and have no export default. The ESM types declare export default sodium with all crypto functions on it. At runtime, crypto functions only exist on the default export (populated after WASM init), so the ESM types are correct and the CJS types are wrong.

Option A: allowSyntheticDefaultImports: true

Tells TypeScript to accept default imports from CJS modules that don't declare one. Solves the immediate problem but is project-wide: any CJS module can now be default-imported without TypeScript catching it. Safe in webpack projects (the bundler handles interop), and very common (implied by esModuleInterop which most projects enable).

Option B: moduleResolution: "node" -> "bundler"

Makes TypeScript respect the package's conditional exports field, so it picks up the ESM types (which correctly declare export default). No need for allowSyntheticDefaultImports. More precise because it fixes the root cause (wrong type declarations being selected) rather than relaxing a global check. Also more correct for a webpack project in general, since "bundler" matches how webpack actually resolves modules.

Wider blast radius though: changes module resolution for every import in the project. Needs verification that nothing else breaks.

@matejvasek

Copy link
Copy Markdown
Collaborator Author

/cc @Cragsmann

@matejvasek

Copy link
Copy Markdown
Collaborator Author

OCP Dynamic Plugin tsconfig Survey

Plugin moduleResolution esModuleInterop allowSyntheticDefaultImports
openshift/console (core) "node" not set not set
console-plugin-template "node" not set not set
lightspeed-console (AI) "node" not set true
console-plugin (Pipelines) "node" not set true
networking-console-plugin "node" true not set
nmstate-console-plugin "node" true not set
kubevirt-plugin "bundler" true true

@matejvasek matejvasek requested a review from Cragsmann June 15, 2026 09:56
@matejvasek matejvasek merged commit 2e57de3 into functions-dev:master Jun 15, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants