Skip to content

Add source-first exports and SSR-safe packaging#143

Open
chrisvire wants to merge 5 commits intosillsdev:mainfrom
chrisvire:feature/svelte-src-package
Open

Add source-first exports and SSR-safe packaging#143
chrisvire wants to merge 5 commits intosillsdev:mainfrom
chrisvire:feature/svelte-src-package

Conversation

@chrisvire
Copy link
Member

@chrisvire chrisvire commented Mar 13, 2026

Summary

This PR updates @ethnolib/language-chooser-svelte-daisyui to support source-first Svelte consumption (SvelteKit-friendly), while preserving dist fallbacks for non-Svelte consumers. It also adds SSR-safety protections and excludes demo/bootstrap code from published library artifacts.

Changes

1) Source-first + fallback exports

Updated components/language-chooser/svelte/language-chooser-svelte-daisyui/package.json:

  • Added:
    • "svelte": "./index.ts"
    • "module": "./dist/index.js"
  • Updated:
    • "main": "./dist/index.cjs"
  • Added exports map for ".":
    • "svelte": "./index.ts"
    • "import": "./dist/index.js"
    • "require": "./dist/index.cjs"
  • Updated publish files:
    • "files": ["dist", "src/lib", "index.ts"]

2) SSR safety checks

Updated components/language-chooser/svelte/language-chooser-svelte-daisyui/src/lib/LanguageChooser.svelte:

  • Added guard before browser-only prompt logic:
    • if (typeof window === "undefined") return;
  • Wrapped alert usage with:
    • if (typeof window !== "undefined") { ... }

This ensures browser globals are only used in browser contexts.

3) Keep demo/bootstrap out of library output

Updated components/language-chooser/svelte/language-chooser-svelte-daisyui/tsconfig.lib.json:

  • Excluded demo files:
    • src/main.ts
    • src/App.svelte
    • src/app.css
  • Narrowed include set to library-facing sources:
    • src/lib/**/*.ts
    • src/lib/**/*.svelte
    • index.ts

Validation

  • Build succeeds:
    • npx vite build in components/language-chooser/svelte/language-chooser-svelte-daisyui
  • Export resolution verified:
    • default import -> dist/index.js
    • --conditions=svelte -> index.ts
    • require -> dist/index.cjs
  • Publish contents verified:
    • npm pack --dry-run does not include src/main.ts
  • Browser global checks:
    • No top-level window/document usage in library entry (components/language-chooser/svelte/language-chooser-svelte-daisyui/index.ts)
    • Browser-only logic is guarded in component code.

Why this matters

  • SvelteKit consumers can compile source directly via the svelte condition.
  • Dist artifacts remain available for non-Svelte toolchains.
  • External runtime behavior and source-first consumption reduce risk of Svelte runtime duplication/conflicts.
  • Published package now excludes demo bootstrap code from the library surface.

Open with Devin

This change is Reviewable

devin-ai-integration[bot]

This comment was marked as resolved.

devin-ai-integration[bot]

This comment was marked as resolved.

@chrisvire chrisvire force-pushed the feature/svelte-src-package branch from 33ebcc7 to eee3c0b Compare March 13, 2026 21:38
Copy link

@devin-ai-integration devin-ai-integration bot left a comment

Choose a reason for hiding this comment

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

Devin Review found 2 new potential issues.

View 4 additional findings in Devin Review.

Open in Devin Review

Comment on lines +125 to +126
value={viewModel.searchString ?? ""}
oninput={onSearchInput}

Choose a reason for hiding this comment

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

🚩 Controlled input pattern replaces bind:value — subtle behavioral difference

The bind:valuevalue={...} + oninput={...} migration (lines 125-126, 192-193) changes from Svelte's native two-way binding to a controlled input pattern. This works correctly because SvelteFieldImpl's setter (field.svelte.ts:19-22) synchronously updates $state before triggering requestUpdate. However, Svelte's bind:value has special internal handling for cursor position preservation that the manual pattern does not. In practice, since the reactive update is synchronous, cursor jumping is unlikely, but it's worth noting this behavioral difference — especially for the search input where rapid typing is common.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Comment on lines +31 to +47
function safeCreateTagFromOrthography(
maybeOrthography: IOrthography
): string | undefined {
const language = maybeOrthography?.language;

// During SSR/initial render, selectedLanguage can be partially populated.
// createTagFromOrthography expects language.scripts to be an array.
if (!language || !Array.isArray(language.scripts)) {
return undefined;
}

try {
return createTagFromOrthography(maybeOrthography);
} catch {
return undefined;
}
}

Choose a reason for hiding this comment

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

🚩 safeCreateTagFromOrthography guards are correct but asymmetric with controller usage

The safeCreateTagFromOrthography wrapper (lines 31-47) guards against SSR scenarios where language or language.scripts might not be properly initialized. This is sound: createTagFromOrthography at languageTagUtils.ts:323 accesses language?.scripts.length which would throw if language exists but scripts is not an array (optional chaining only protects against language being nullish, not scripts). However, it's worth noting that the controller's _updateTagPreview (language-chooser.ts:181) calls createTagFromOrthography directly without this guard. If SSR ever reaches the controller's code path, the same crash could occur there. The current guard only protects the Svelte component's $derived expression.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

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.

1 participant