You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Post this to aehrc/smart-formsfirst, before any of the three branches, so the
maintainers can respond to the design before reviewing code. Their CONTRIBUTING.md asks for
discussion ahead of significant work, and unlike the four dependency PRs already open, these
changes are significant.
Title: Support non-DOM consumers of the engine (React Native, Node, workers)
Labels: suggest enhancement. No existing label covers packaging.
Body
@aehrc/smart-forms-renderer bundles two things that are useful separately: an SDC
questionnaire engine, and a Material UI renderer for it. Today only the second is reachable
through a supported path, and that makes the first hard to reuse.
The problem
The package publishes main: lib/index.js with no exports map and no sideEffects field.
Importing anything from the barrel pulls in @mui/material, @emotion/*, react-dom, react-markdown and both drag-and-drop libraries, none of which run outside a browser.
So a consumer that wants the engine without a DOM has no supported entry point. In practice
it deep-imports internal paths instead, for example:
Those are not public API. They can move in a patch release, and importing from a lib/utils
barrel rather than a specific file drags the whole re-export graph in, which is how html-react-parser ends up in a React Native bundle. A consumer doing this has to pin the
package to an exact version and keep its own guard tests to detect the UI layer creeping back
in on every upgrade.
This affects web consumers too: with no exports map and no sideEffects flag, a bundler
cannot tree-shake the parts of the package an app does not use.
Why this is packaging, not a rewrite
The engine and the UI are already almost cleanly separated in the source. Across src/utils, src/stores and src/api, only two modules reach the UI layer at all:
src/utils/extensions.ts imports html-react-parser and calls htmlParse at exactly one
site, inside getTextDisplayFlyover. That makes a React-element-returning function part of
the core extension reader.
src/stores/rendererConfigStore.ts has a type-only@mui/material import for Breakpoints, which is erased at runtime.
Everything else in those directories is already DOM-free. So exposing the engine is mostly a
matter of deciding what the supported surface is and declaring it.
Proposed direction
Three changes, smallest first:
Move the htmlParse call out of the core extension reader. Have utils/extensions.ts return the raw xhtml string and let the UI layer parse it; hooks/useParseXhtml.tsx already exists and is the natural home. This is the prerequisite
for the rest.
Make the terminology transport injectable.utils/valueSet.ts and utils/questionnaireStoreUtils/addDisplayToCodings.ts each construct a fhirclient
inline for $expand, $validate-code and $lookup, and resolveValueSetPromises has a
hardcoded 5000ms timeout. There is no seam for a consumer to supply its own transport, so a
host with its own authenticated terminology client has to bypass the engine's value set
path entirely, losing answerValueSet handling, $validate-code and addDisplayToCodings. @aehrc/sdc-populate and @aehrc/sdc-assemble already take
injected fetch callbacks, so this is consistency with the sibling packages rather than a
new idea.
Add an exports map with a curated headless entrypoint (./engine), plus sideEffects: false once the barrel's @fontsource/inter side effect is gone (that is
already open as Remove the webfont side effect from the renderer entrypoint #2053). The map must keep a ./lib/* passthrough, because an exports map
is a whitelist and removing those paths would break every consumer currently deep-importing
them.
Groundwork already open
The four small PRs open now are the dependency-hygiene half of the same goal, and each stands
on its own:
No React Native UI layer upstream. The engine seams are the reusable part. A second
widget set would be a maintenance burden for the team with only one consumer, and it is not
being asked for. The request is only that the engine be reachable; drawing it stays the host's
problem.
Context on the ask
Each of the three has its own issue and pull request, opened alongside this one so the code is
there to look at rather than described in the abstract. They are independent changes and none
of them has to land as part of a bigger plan. This issue exists to give them a shared rationale
and to ask the design questions that span them, particularly:
Whether a ./engine subpath on the existing package is preferred over a separate @aehrc/smart-forms-core package.
What belongs in that supported surface. My branch derives it from one real consumer's
imports, which is a starting point rather than a considered API.
Whether the ./lib/* passthrough should be permanent or deprecated over a release or two.
For the terminology seam, whether it belongs on terminologyServerStore (which already
owns the server URL) or on rendererConfigStore.
Happy to split, reshape or drop any of it. Nothing here needs to land as a single change.
Draft umbrella issue (ready to post)
Post this to
aehrc/smart-formsfirst, before any of the three branches, so themaintainers can respond to the design before reviewing code. Their CONTRIBUTING.md asks for
discussion ahead of significant work, and unlike the four dependency PRs already open, these
changes are significant.
Title: Support non-DOM consumers of the engine (React Native, Node, workers)
Labels: suggest
enhancement. No existing label covers packaging.Body
@aehrc/smart-forms-rendererbundles two things that are useful separately: an SDCquestionnaire engine, and a Material UI renderer for it. Today only the second is reachable
through a supported path, and that makes the first hard to reuse.
The problem
The package publishes
main: lib/index.jswith noexportsmap and nosideEffectsfield.Importing anything from the barrel pulls in
@mui/material,@emotion/*,react-dom,react-markdownand both drag-and-drop libraries, none of which run outside a browser.So a consumer that wants the engine without a DOM has no supported entry point. In practice
it deep-imports internal paths instead, for example:
Those are not public API. They can move in a patch release, and importing from a
lib/utilsbarrel rather than a specific file drags the whole re-export graph in, which is how
html-react-parserends up in a React Native bundle. A consumer doing this has to pin thepackage to an exact version and keep its own guard tests to detect the UI layer creeping back
in on every upgrade.
This affects web consumers too: with no
exportsmap and nosideEffectsflag, a bundlercannot tree-shake the parts of the package an app does not use.
Why this is packaging, not a rewrite
The engine and the UI are already almost cleanly separated in the source. Across
src/utils,src/storesandsrc/api, only two modules reach the UI layer at all:src/utils/extensions.tsimportshtml-react-parserand callshtmlParseat exactly onesite, inside
getTextDisplayFlyover. That makes a React-element-returning function part ofthe core extension reader.
src/stores/rendererConfigStore.tshas a type-only@mui/materialimport forBreakpoints, which is erased at runtime.Everything else in those directories is already DOM-free. So exposing the engine is mostly a
matter of deciding what the supported surface is and declaring it.
Proposed direction
Three changes, smallest first:
htmlParsecall out of the core extension reader. Haveutils/extensions.tsreturn the raw xhtml string and let the UI layer parse it;hooks/useParseXhtml.tsxalready exists and is the natural home. This is the prerequisitefor the rest.
utils/valueSet.tsandutils/questionnaireStoreUtils/addDisplayToCodings.tseach construct afhirclientinline for
$expand,$validate-codeand$lookup, andresolveValueSetPromiseshas ahardcoded 5000ms timeout. There is no seam for a consumer to supply its own transport, so a
host with its own authenticated terminology client has to bypass the engine's value set
path entirely, losing
answerValueSethandling,$validate-codeandaddDisplayToCodings.@aehrc/sdc-populateand@aehrc/sdc-assemblealready takeinjected fetch callbacks, so this is consistency with the sibling packages rather than a
new idea.
exportsmap with a curated headless entrypoint (./engine), plussideEffects: falseonce the barrel's@fontsource/interside effect is gone (that isalready open as Remove the webfont side effect from the renderer entrypoint #2053). The map must keep a
./lib/*passthrough, because anexportsmapis a whitelist and removing those paths would break every consumer currently deep-importing
them.
Groundwork already open
The four small PRs open now are the dependency-hygiene half of the same goal, and each stands
on its own:
axiosruntime dependency fromsdc-assemble@fontsource/interside effect from the renderer entrypointreact-beautiful-dndwith@hello-pangea/dndmomentwithdayjsinsdc-populateScope, and what is deliberately not proposed
No React Native UI layer upstream. The engine seams are the reusable part. A second
widget set would be a maintenance burden for the team with only one consumer, and it is not
being asked for. The request is only that the engine be reachable; drawing it stays the host's
problem.
Context on the ask
Each of the three has its own issue and pull request, opened alongside this one so the code is
there to look at rather than described in the abstract. They are independent changes and none
of them has to land as part of a bigger plan. This issue exists to give them a shared rationale
and to ask the design questions that span them, particularly:
./enginesubpath on the existing package is preferred over a separate@aehrc/smart-forms-corepackage.imports, which is a starting point rather than a considered API.
./lib/*passthrough should be permanent or deprecated over a release or two.terminologyServerStore(which alreadyowns the server URL) or on
rendererConfigStore.Happy to split, reshape or drop any of it. Nothing here needs to land as a single change.