Repository: effector/effector
Pull request #1063 (2024-04-14 12:15:16)

Region derived sids proto


Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index b24f23ee..2168d57a 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -7,6 +7,7 @@ See also [separate changelogs for each library](https://changelog.effector.dev/)
 - Delete deprecated option `reactSsr` in babel-plugin ([PR #1084](https://github.com/effector/effector/pull/1084))
 - Delete deprecated operator `forward` ([PR #1087](https://github.com/effector/effector/pull/1087))
 - Delete deprecated operator `guard` ([PR #1087](https://github.com/effector/effector/pull/1087))
+- Improve SIDs generation mechanic: if unit is created by a registered factory, its SID will be derived from the SID of the factory ([PR #1063](https://github.com/effector/effector/pull/1063))
 
 ## effector-react 24.0.0
 
diff --git a/src/effector/region.ts b/src/effector/region.ts
index 4a8e0ead..305ff434 100644
--- a/src/effector/region.ts
+++ b/src/effector/region.ts
@@ -19,7 +19,8 @@ type RegionStack = {
   parent: RegionStack | null
   value: any
   template: Template | null
-  sidRoot?: string
+  sidRoot?: string | null
+  sidOffset: number
   meta:
     | Record<string, unknown>
     | {
@@ -42,9 +43,24 @@ export const reportDeclaration = (node: Node | 'region') => {
 export const readTemplate = (): Template | null =>
   regionStack && regionStack.template
 export const readSidRoot = (sid?: string | null) => {
-  if (sid && regionStack && regionStack.sidRoot)
-    sid = `${regionStack.sidRoot}|${sid}`
-  return sid
+  /**
+   * If there is no regionStack or sidRoot in regionStack, return sid as is (including null and undefined cases)
+   */
+  if (!regionStack || !regionStack.sidRoot) return sid
+ 
+  /**
+   * If sid is provided, return sidRoot + sid
+   */
+  if (sid) {
+    return `${regionStack.sidRoot}|${sid}|${regionStack.sidOffset++}`
+  }
+
+  /**
+   * If sid is not provided, but region stack is available:
+   * 
+   * Try to derive sid from regionStack.sidRoot and regionStack.sidOffset.
+   */
+  return `${regionStack.sidRoot}|${regionStack.sidOffset++}`
 }
 
 export function withRegion<T = void>(unit: NodeUnit, cb: () => T): T {
@@ -55,8 +71,9 @@ export function withRegion<T = void>(unit: NodeUnit, cb: () => T): T {
     parent: regionStack,
     value: unit,
     template: meta.template || readTemplate(),
-    sidRoot: meta.sidRoot || (regionStack && regionStack.sidRoot),
+    sidRoot: readSidRoot(meta.sidRoot),
     meta: meta,
+    sidOffset: 0,
   }
   try {
     return cb()


Test patch:
diff --git a/src/effector/__tests__/fork/factory.test.ts b/src/effector/__tests__/fork/factory.test.ts
index a16c04f1..30188f86 100644
--- a/src/effector/__tests__/fork/factory.test.ts
+++ b/src/effector/__tests__/fork/factory.test.ts
@@ -1,5 +1,5 @@
-import {fork, serialize} from 'effector'
-import {createField, createFieldset} from './factory'
+import {allSettled, createEvent, createStore, fork, serialize} from 'effector'
+import {createField, createFieldset, topLevelFactory} from './factory'
 
 test('factory support', async () => {
   const username = createField('username', 'guest')
@@ -12,8 +12,8 @@ test('factory support', async () => {
   })
   expect(serialize(scope)).toMatchInlineSnapshot(`
     Object {
-      "-iajnln|-77rc2s": 21,
-      "8iua16|-77rc2s": "alice",
+      "-iajnln|-77rc2s|2": 21,
+      "8iua16|-77rc2s|2": "alice",
     }
   `)
 })
@@ -31,8 +31,43 @@ test('nested factory support', async () => {
   })
   expect(serialize(scope)).toMatchInlineSnapshot(`
     Object {
-      "-fjbluz|1104zu|-77rc2s": "alice",
-      "-fjbluz|11jxl7|-77rc2s": 21,
+      "-fjbluz|-fjbluz|1104zu|1|2|-77rc2s|2": "alice",
+      "-fjbluz|-fjbluz|11jxl7|3|4|-77rc2s|2": 21,
+    }
+  `)
+})
+
+test('derived sids support', async () => {
+  const pushUpdate = createEvent()
+  const inlineFactory = (initial: number) => {
+    const $interalStore = createStore(initial, {
+      // not-unique sid
+      sid: '$interalStore',
+    }).on(pushUpdate, state => state + 1)
+
+    return {
+      $interalStore,
+    }
+  }
+
+  topLevelFactory(() => {
+    inlineFactory(1)
+    inlineFactory(2)
+
+    topLevelFactory(() => {
+      inlineFactory(3)
+    })
+  })
+
+  const scope = fork()
+
+  await allSettled(pushUpdate, {scope})
+
+  expect(serialize(scope)).toMatchInlineSnapshot(`
+    Object {
+      "ot2oxd|$interalStore|1": 2,
+      "ot2oxd|$interalStore|4": 3,
+      "ot2oxd|ot2oxd|ov9vcj|6|7|$interalStore|1": 4,
     }
   `)
 })
diff --git a/src/effector/__tests__/fork/factory.ts b/src/effector/__tests__/fork/factory.ts
index f93a130a..0925a465 100644
--- a/src/effector/__tests__/fork/factory.ts
+++ b/src/effector/__tests__/fork/factory.ts
@@ -18,3 +18,7 @@ export function createFieldset(fn: () => ReturnType<typeof createField>[]) {
   const fieldset = combine(shape)
   return {shape, fieldset, reset}
 }
+
+export function topLevelFactory(fn: any) {
+  return fn()
+}