feat(core,methods): add react-native framework adapter (#117)#137
feat(core,methods): add react-native framework adapter (#117)#137espetro wants to merge 3 commits into
Conversation
Extends the `Framework` union with 'react-native' and adds a `index.react-native.ts` adapter identical to the React one (`createSignal`, `batch`, `untrack`, `createId`) with only the `framework` constant changed. This is the signal-layer foundation for a future `@formisch/react-native` package; browser-specific overrides (`FieldElement`, `getElementInput`) are intentionally out of scope here. Refs: open-circle#117
Adds 'react-native' to the per-framework build matrix in both
`packages/core/tsdown.config.ts` and `packages/methods/tsdown.config.ts`,
and exposes `./react-native` in both packages' export maps. The
`rewriteFrameworkImports` tsdown plugin rewires
`@formisch/core` -> `@formisch/core/react-native` in the methods
sources for the RN target, so no plugin work is required.
Produces `dist/index.react-native.{js,d.ts}` in both packages.
Refs: open-circle#117
|
@espetro is attempting to deploy a commit to the Open Circle Team on Vercel. A member of the Team first needs to authorize it. |
dosubot
Bot
added
size:L
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
WalkthroughThis PR adds React Native as a supported framework target across the formisch monorepo. A new file Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/core/src/framework/index.react-native.ts`:
- Line 56: The createSignal function is missing the zero-argument overload that
exists in the base framework contract. In both index.react-native.ts and
index.react.ts, add an overload declaration for createSignal that accepts no
parameters and returns Signal<T>, in addition to the existing overload that
takes an initial value parameter. This ensures both adapters match the complete
API contract defined in the base framework, allowing users to call
createSignal() with no arguments as well as with an initial value.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3797d810-21ca-48d1-99fc-c874f64b76c3
📒 Files selected for processing (6)
packages/core/package.jsonpackages/core/src/framework/index.react-native.tspackages/core/src/framework/index.tspackages/core/tsdown.config.tspackages/methods/package.jsonpackages/methods/tsdown.config.ts
| * @returns The created signal. | ||
| */ | ||
| // @__NO_SIDE_EFFECTS__ | ||
| export function createSignal<T>(value: T): Signal<T> { |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify createSignal signature consistency across framework entrypoints and call sites.
fd -i 'index*.ts' packages/core/src/framework
rg -nP --type=ts '^export function createSignal<.*>\(\): Signal<.*>;' packages/core/src/framework -C2
rg -nP --type=ts '^export function createSignal<.*>\(value: .*' packages/core/src/framework -C2
rg -nP --type=ts '\bcreateSignal\s*(<[^>]+>)?\s*\(\s*\)' packages -C2Repository: open-circle/formisch
Length of output: 2059
createSignal is missing the zero-arg overload from the base framework API.
The base contract in packages/core/src/framework/index.ts declares two overloads for createSignal—one without arguments and one with an initial value. At line 56 in the react-native adapter, only the required-argument overload is exposed, creating an API mismatch. The same issue exists in index.react.ts.
Suggested fix
// `@__NO_SIDE_EFFECTS__`
+export function createSignal<T>(): Signal<T | undefined>;
export function createSignal<T>(value: T): Signal<T>;
-export function createSignal<T>(value: T): Signal<T | undefined> {
+export function createSignal<T>(value?: T): Signal<T | undefined> {
const subscribers = new Set<Listener>();
return {
get value() {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/core/src/framework/index.react-native.ts` at line 56, The
createSignal function is missing the zero-argument overload that exists in the
base framework contract. In both index.react-native.ts and index.react.ts, add
an overload declaration for createSignal that accepts no parameters and returns
Signal<T>, in addition to the existing overload that takes an initial value
parameter. This ensures both adapters match the complete API contract defined in
the base framework, allowing users to call createSignal() with no arguments as
well as with an initial value.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 6 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/core/src/framework/index.react-native.ts">
<violation number="1" location="packages/core/src/framework/index.react-native.ts:7">
P2: React Native adapter duplicates the entire React adapter signal implementation. The two files differ only by the `framework` constant, creating a maintenance burden and drift risk for any future signal-layer changes.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| @@ -0,0 +1,131 @@ | |||
| import type { Signal } from '../types/signal/index.ts'; | |||
There was a problem hiding this comment.
P2: React Native adapter duplicates the entire React adapter signal implementation. The two files differ only by the framework constant, creating a maintenance burden and drift risk for any future signal-layer changes.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/core/src/framework/index.react-native.ts, line 7:
<comment>React Native adapter duplicates the entire React adapter signal implementation. The two files differ only by the `framework` constant, creating a maintenance burden and drift risk for any future signal-layer changes.</comment>
<file context>
@@ -0,0 +1,131 @@
+/**
+ * The current framework being used.
+ */
+export const framework: Framework = 'react-native';
+
+/**
</file context>
fabian-hiller
left a comment
fabian-hiller
left a comment
There was a problem hiding this comment.
Thank you for this PR! It would be great to offer Formisch for React Native within the next weeks.
I think we should do what you marked as "Out of scope" in the PR description as part of this PR. Since React Native is a special case it would be great to confirm that what we have in mind make sense and works with React Native. A bigger PR is ok in such a case. I just want to prevent merging something that can not be verified and tested.
Maybe I am wrong but there might be a chance that the only change we need is setting the FieldElement type to e.g. React Native's TextInput, Switch, and so on. FieldElement is only used in InternalBaseStore and getElementInput. Since getElementInput is not used anywhere else within the core and methods package, it can probably be ignored.
After that, we need a new frameworks/react-native and playgrounds/react-native folder with React Native specific code.
| /** | ||
| * The current framework being used. | ||
| */ | ||
| export const framework: Framework = 'react-native'; |
There was a problem hiding this comment.
If this file is fully identically to index.react.ts we could try re-exporting the functions of index.react.ts so that we only have to maintain one implementation. Just a spontaneous idea we should at least test and consider. But this is not a blocker.
There was a problem hiding this comment.
I'll check it, nevertheless take into account that the development teams at react and react-native don't sync on their releases, and this could create issues in the future. For instance, the React team announced the v19 release in Dec. 2024 and it took the React Native team until Feb. 2025 to ship it. That's 3 months worth of potential bugs (now with AI-assisted development it'd be much less of course, but it still implies some risk)
|
Hey @fabian-hiller before I proceed with the changes, I want to get your approval on one topic: when looking how to decouple core types from DOM types, I noticed the easiest fix was to change InternalBaseStore.elements and initialElements from Thus, instead I'd like to suggest a minimal structural base type: At
|
2 participants
Summary
Adds the React Native framework adapter to
@formisch/coreand wires up a React Native build target in both@formisch/coreand@formisch/methods, per #117. This is the signal-layer foundation only — purely additive, no behaviour changes.Changes
packages/core/src/framework/index.ts— add'react-native'to theFrameworkunionpackages/core/src/framework/index.react-native.ts(NEW) — adapter identical toindex.react.tswithframework = 'react-native'packages/core/package.json— expose./react-nativeexportpackages/core/tsdown.config.ts— add'react-native'to the build matrix (added during impl; see note below)packages/methods/tsdown.config.ts— add'react-native'to the build matrixpackages/methods/package.json— expose./react-nativeexportWhy
React Native shares React's rendering core, so the signal/batch/untrack primitives transfer verbatim. The existing
rewriteFrameworkImportstsdown plugin (confirmed by @fabian-hiller in #117) auto-rewires@formisch/core→@formisch/core/react-nativefor the RN build target, so no plugin work is needed.Out of scope (follow-up)
Tracked for a separate
frameworks/react-nativePR:FieldElementtype andgetElementInputRN-specific overrides inpackages/coreuseFieldevent-handler adaptation (onChangeText/field.onChangeper maintainer guidance)Formcomponent (View vs form element) andField/TextInput/Switch/Pickeradaptersframeworks/react-nativepackage, tests, and playgroundPlan brief on the fork: espetro/formisch#1
Verification
pnpm -C packages/core build→ producesdist/index.react-native.{js,d.ts}✅pnpm -C packages/methods build→ producesdist/index.react-native.{js,d.ts}✅pnpm -C packages/core lint→ clean ✅pnpm -C packages/methods lint→ clean ✅pnpm -C packages/core test→ 384 tests pass, 0 type errors ✅pnpm -C packages/methods test→ 157 tests pass, 0 type errors ✅Commits
feat(core): add react-native framework adapter—packages/core/src/framework/index.ts+ newindex.react-native.tsfeat(core,methods): expose react-native build output— 4 package.json + tsdown.config.ts filesCloses #117
Summary by cubic
Adds a React Native framework adapter and React Native build targets for
@formisch/coreand@formisch/methods. This is the signal-layer foundation only with no behavior changes.'react-native'to theFrameworkunion and a newpackages/core/src/framework/index.react-native.tsadapter (same as React, withframework = 'react-native'andcreateSignal,batch,untrack,createId)../react-nativeexports and added React Native to the tsdown build matrix in both packages, producingdist/index.react-native.{js,d.ts}as required by Add React Native framework adapter #117.Written for commit eea4c1c. Summary will update on new commits.