fix: make module importable without a DOM (Node/SSR) so calculate() works#17
Open
dmchaledev wants to merge 1 commit into
Open
fix: make module importable without a DOM (Node/SSR) so calculate() works#17dmchaledev wants to merge 1 commit into
dmchaledev wants to merge 1 commit into
Conversation
The module ran document.createElement() at the top level and extended HTMLElement / called customElements.define() unconditionally, so importing it threw 'ReferenceError: document is not defined' in any non-browser environment. This broke the exported calculate() function — documented as 'no DOM required' — for Node consumers and crashed server-side rendering (Next.js/Astro/etc.) at build time. - Build the <template> lazily on first component construction instead of at module load. - Extend a plain base class when HTMLElement is undefined. - Guard customElements.define() behind a browser-environment check. Adds a regression test that spawns a fresh, unshimmed Node process to verify the module imports and calculate() runs without any DOM globals. The existing DOM-shimmed tests masked this, so the guard runs in a separate process and is included in the default npm test run.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The package exports a pure
calculate()function, documented inindex.d.tsas:But the module cannot actually be imported without a DOM. Three things run at module-load time and reference browser globals:
const TMPL = document.createElement('template')(top level) →ReferenceError: document is not definedclass HailbytesVulnCalculator extends HTMLElement→HTMLElementis undefined in NodecustomElements.define(...)(top level) →customElementsis undefined in NodeReproduced against
main:Impact
calculate()API is unusable in Node, scripts, or tests without manually constructing a fake DOM.imports the package breaks the build.smoke.test.mjsandcalculate.test.mjsinstall a full DOM shim onglobalThisbefore importing, so CI never exercises the real-world "no DOM" import path.This is distinct from the input-robustness work in #14 (which handles omitted inputs); here the module can't even be loaded.
Fix
Defer all DOM access out of module-load time — the web component still behaves identically in the browser:
<template><template>on first component construction viagetTemplate()instead of at top level.extends (typeof HTMLElement !== 'undefined' ? HTMLElement : class {})so the class declaration doesn't throw in Node.customElements.define(...)only runs whencustomElements/HTMLElementexist.No calculation logic changed; the browser code path is unchanged (template is built the first time the element is constructed, before
connectedCallback).Verification
Regression test
Added a guard to
smoke.test.mjsthat spawns a fresh, unshimmed Node process to import the module and runcalculate()with no DOM globals present. Because it runs in a separate process, the DOM shim used by the other tests can't hide a regression — and it's part of the defaultnpm test(which currently runs only the smoke suite).Self-contained: source + one regression test, no API or behavior changes for existing consumers.
https://claude.ai/code/session_01YGoUBT4BGcWuGqYdVcQRHT
Generated by Claude Code