Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
108 changes: 108 additions & 0 deletions components/state-management/state-management-react/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
# State Management Core

This package provides a simple interface for working with multiple reactive framworks.
Copy link

Copilot AI Dec 10, 2025

Choose a reason for hiding this comment

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

Typo: "framworks" should be "frameworks"

Suggested change
This package provides a simple interface for working with multiple reactive framworks.
This package provides a simple interface for working with multiple reactive frameworks.

Copilot uses AI. Check for mistakes.

It assumes a view/view model architecture for your app. The view is responsible for what
the user sees, and the view model is responsible for the stateful logic that makes the view
operational.

This package establishes a convention for writing a view model in vanilla JavaScript and
your view in React, Svelte, or some other reactive framework.

This package exists for view models that are exposed as libraries. Since your view model
is not tied to a specific framework, developers can write their own views using their
framework of choice.

## Fields

A field represents a single piece of reactive data. Its usage is best explained with an example.

Suppose your app has a search bar. Every time the user changes the search text, the app
should perform a search.

Here's how you would define the search text field:

```js
const searchText = new Field("", (newSearchText) => {
// This callback runs whenever the user changes the search text.
searchFor(newSearchText);
});

// Get current value of the field
let text = searchText.value;

// Update the field and perform the search.
// This is how the UI should update the field.
searchText.requestUpdate("hello world");

// Update the field without performing the search.
// Your program can use this method internally to update the field without side effects.
searchText.value = "abc";
```

## View Models

In the context of this package, a view model is an object with fields.
To define a view model, write a function like the following:

```js
function usePersonViewModel() {
// The view model can have fields...
const name = new Field();
const age = new Field();

// ...and actions
function haveBirthday() {
age.value++;
}

return { name, age, haveBirthday };
}
```

## Usage in UI framework

Framework-specific adapters make view models accessible to your UI. For example,
if you framework is Svelte, use the `state-management-svelte` package:

```svelte
<script>
import { svelteViewModel } from "@ethnolib/state-management-svelte";

// svelteViewModel() turns all fields into reactive Svelte properties
const person = svelteViewModel(usePersonViewModel())

person.name = "John" // Behind the scenes, this calls `requestUpdate`
</script>

<p>Hello, {person.name}!</p>
```

For more details, see documentation for `state-management-svelte` or the adapter for your
framework of choice.

Behind the scenes, the adapter does something like this to keep the view model in sync with the UI:

```js
const person = usePersonViewModel();

// Replace `defineReactiveState` with your framework's mechanism
const reactiveName = defineReactiveSate();
Copy link

Copilot AI Dec 10, 2025

Choose a reason for hiding this comment

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

Typo: "defineReactiveSate" should be "defineReactiveState"

Suggested change
const reactiveName = defineReactiveSate();
const reactiveName = defineReactiveState();

Copilot uses AI. Check for mistakes.

/**
* Establish a two-way binding between `reactiveName` and `person.name`:
*/

// Update the UI in response to the field
person.name.updateUI = (value) => {
reactiveName = value; // Or however you update reactiveName in your framework
};

// Update the field in response to the UI.
// Replace `watch()` with whatever your framework uses to subscribe to a variable.
watch(reactiveName, (value) => person.name.requestUpdate(value));

/**
* Now your UI can use `reactiveName` to interact with the name field.
*/
```
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export * from "./src/use-field";
28 changes: 28 additions & 0 deletions components/state-management/state-management-react/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
{
"name": "@ethnolib/state-management-react",
"description": "An adapter to use @ethnolib/state-management-core with React",
"author": "SIL Global",
"license": "MIT",
"version": "0.1.0",
"main": "./index.js",
"types": "./index.d.ts",
"scripts": {
"build": "nx vite:build",
"typecheck": "tsc",
"test": "nx vite:test --config vitest.config.ts",
"testonce": "nx vite:test --config vitest.config.ts --run",
"lint": "eslint ."
},
"devDependencies": {
"@nx/vite": "^19.1.2",
"@types/react": "^17",
"@types/react-dom": "^17",
"@types/node": "^20.16.11",
"@vitejs/plugin-react-swc": "^3.8.0",
"tsx": "^4.19.2",
"typescript": "^5.2.2"
},
Copy link

Copilot AI Dec 10, 2025

Choose a reason for hiding this comment

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

Missing peerDependencies for "react" (and optionally "react-dom"). Since this package is a React adapter that imports from "react", it should declare React as a peer dependency to ensure consumers have it installed. Consider adding:

"peerDependencies": {
  "react": "^17.0.0 || ^18.0.0"
}
Suggested change
},
},
"peerDependencies": {
"react": "^17.0.0 || ^18.0.0"
},

Copilot uses AI. Check for mistakes.
"volta": {
"extends": "../../../package.json"
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
import { useState } from "react";
import { Field } from "@ethnolib/state-management-core";

export function useField<T>(field: Field<T>): [T, (value: T) => void] {
const [fieldValue, _setFieldValue] = useState(field.value);

function setFieldValue(value: T) {
field.requestUpdate(value);
_setFieldValue(value);
}
Comment on lines +7 to +10

Choose a reason for hiding this comment

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

🔴 setFieldValue calls _setFieldValue redundantly, risking state desync if onUpdateRequested modifies the field

In setFieldValue, _setFieldValue(value) on line 9 is called after field.requestUpdate(value) on line 8. However, requestUpdate already triggers _setFieldValue(value) via the updateUI callback (see components/state-management/state-management-core/src/field.ts:23-28requestUpdate sets this.value, whose setter at line 42 calls this.updateUI(value)).

How this causes state desync

The call chain when setFieldValue(value) is invoked:

  1. field.requestUpdate(value)this.value = valuethis.updateUI(value)_setFieldValue(value)
  2. _onUpdateRequested(value, oldValue) runs — if this callback modifies field.value to a different value (e.g., normalization/validation), that triggers updateUI(correctedValue)_setFieldValue(correctedValue)
  3. Back in setFieldValue, line 9 calls _setFieldValue(value) with the original value, overwriting the corrected value from step 2.

While current usages of _onUpdateRequested in the codebase don't modify the same field they're attached to, this is a latent bug that will manifest as soon as any onUpdateRequested callback normalizes or validates the field's own value. The Svelte adapter (components/state-management/state-management-svelte/src/field.svelte.ts:19-22) does not have this issue because it sets _state before calling requestUpdate, and the redundant updateUI call sets _state to the same value.

The fix is to remove line 9 (_setFieldValue(value)) since requestUpdate already triggers the UI update via updateUI.

Suggested change
function setFieldValue(value: T) {
field.requestUpdate(value);
_setFieldValue(value);
}
function setFieldValue(value: T) {
field.requestUpdate(value);
}
Open in Devin Review

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


field.updateUI = (value) => _setFieldValue(value);
Copy link

Copilot AI Dec 10, 2025

Choose a reason for hiding this comment

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

The field.updateUI assignment on every render can cause issues. If the component re-renders (e.g., due to parent state changes), this assignment will overwrite the callback, potentially breaking synchronization. Consider using useEffect to set this up once:

useEffect(() => {
  field.updateUI = (value) => _setFieldValue(value);
  return () => {
    field.updateUI = null; // cleanup
  };
}, [field]);

Copilot uses AI. Check for mistakes.

Choose a reason for hiding this comment

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

🔴 Missing useEffect for field.updateUI assignment causes stale callback after unmount

The field.updateUI callback is assigned directly during render (line 12) rather than inside a useEffect. This means:

  1. No cleanup on unmount: When the component using useField unmounts, field.updateUI still references the stale _setFieldValue from the unmounted component. If anything later sets field.value (which triggers updateUI via the setter at components/state-management/state-management-core/src/field.ts:42), it will call setState on an unmounted component.

  2. Side effect during render: Assigning field.updateUI mutates an external object during the render phase, violating React's rules about pure rendering.

Detailed explanation and comparison with Svelte adapter

The Svelte adapter (components/state-management/state-management-svelte/src/field.svelte.ts:9) sets field.updateUI in the constructor, which is appropriate for Svelte's lifecycle. However, in React, side effects that interact with external state must be placed in useEffect with proper cleanup.

The fix should wrap the assignment in useEffect and return a cleanup function:

useEffect(() => {
  field.updateUI = (value) => _setFieldValue(value);
  return () => { field.updateUI = null; };
}, [field]);

Without this, if a component unmounts and the Field object persists (which is the expected pattern — Fields live in view models that outlive individual React components), subsequent field.value = x calls will attempt to update React state on an unmounted component. While React 18+ silently ignores this, it indicates a real resource leak where the Field holds a reference to a dead component's state setter.

Prompt for agents
In components/state-management/state-management-react/src/use-field.ts, the field.updateUI assignment on line 12 should be moved into a useEffect hook with proper cleanup. Add useEffect to the import from react on line 1, then replace line 12 with:

useEffect(() => {
  field.updateUI = (value) => _setFieldValue(value);
  return () => { field.updateUI = null; };
}, [field]);

This ensures the callback is properly set up after mount and cleaned up on unmount, preventing stale references to the setState function of an unmounted component.
Open in Devin Review

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


return [fieldValue, setFieldValue];
}
Comment on lines +4 to +15
Copy link

Copilot AI Dec 10, 2025

Choose a reason for hiding this comment

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

The useField hook lacks test coverage. The sibling package state-management-svelte has tests (e.g., transform-view-model.test.ts) for similar functionality. Consider adding tests to verify:

  • Initial state matches field value
  • Updates through setFieldValue call field.requestUpdate
  • Updates through field.updateUI update React state
  • Cleanup of field.updateUI on unmount

Copilot uses AI. Check for mistakes.
22 changes: 22 additions & 0 deletions components/state-management/state-management-react/tsconfig.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
{
"compilerOptions": {
"jsx": "react-jsx",
"allowJs": false,
"esModuleInterop": false,
"allowSyntheticDefaultImports": true,
"strict": true
},
"ts-node": {
"moduleTypes": {
"*": "esm"
}
},
"files": [],
"include": [],
"references": [
{
"path": "./tsconfig.lib.json"
}
],
"extends": "../../../tsconfig.base.json"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
{
"extends": "./tsconfig.json",
"compilerOptions": {
"outDir": "../../../dist/out-tsc",
"types": ["node", "vite/client"],
"composite": true,
"declaration": true,
"declarationMap": true
},
"exclude": [
"langtagProcessing.ts",
Copy link

Copilot AI Dec 10, 2025

Choose a reason for hiding this comment

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

The exclude pattern references "langtagProcessing.ts" which doesn't exist in this package. This appears to be a leftover from a template and should be removed.

Suggested change
"langtagProcessing.ts",

Copilot uses AI. Check for mistakes.
"**/*.spec.ts",
"**/*.test.ts",
"**/*.spec.tsx",
"**/*.test.tsx",
"**/*.spec.js",
"**/*.test.js",
"**/*.spec.jsx",
"**/*.test.jsx"
],
"include": [
"./**/*.js",
"./**/*.jsx",
"./**/*.ts",
"./**/*.tsx",
"../index.ts"
]
}
36 changes: 36 additions & 0 deletions components/state-management/state-management-react/vite.config.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
/// <reference types='vitest' />
import { defineConfig } from "vite";
import dts from "vite-plugin-dts";
import * as path from "path";
import { nxViteTsPaths } from "@nx/vite/plugins/nx-tsconfig-paths.plugin";

export default defineConfig({
root: __dirname,
cacheDir:
"../../../node_modules/.vite/components/state-management/state-management-core",
Copy link

Copilot AI Dec 10, 2025

Choose a reason for hiding this comment

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

The cacheDir path references "state-management-core" but should reference "state-management-react" to match the actual package being built.

Suggested change
"../../../node_modules/.vite/components/state-management/state-management-core",
"../../../node_modules/.vite/components/state-management/state-management-react",

Copilot uses AI. Check for mistakes.

plugins: [
nxViteTsPaths(),
dts({
entryRoot: ".",
tsconfigPath: path.join(__dirname, "tsconfig.lib.json"),
}),
],

// Configuration for building your library.
// See: https://vitejs.dev/guide/build.html#library-mode
build: {
outDir: "./dist",
emptyOutDir: true,
reportCompressedSize: true,
commonjsOptions: {
transformMixedEsModules: true,
},
lib: {
entry: "./index.ts",
name: "@ethnolib/find-language",
Copy link

Copilot AI Dec 10, 2025

Choose a reason for hiding this comment

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

The library name is incorrectly set to "@ethnolib/find-language". It should be "@ethnolib/state-management-react" to match the package name.

Suggested change
name: "@ethnolib/find-language",
name: "@ethnolib/state-management-react",

Copilot uses AI. Check for mistakes.
fileName: "index",
formats: ["es", "cjs"],
},
},
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
/// <reference types="vitest" />
import { defineConfig } from "vite";

export default defineConfig({
test: {
expect: {
requireAssertions: true,
},
},
});
Loading