Refactor cb-4018 data context api #2708

Wroud · 2024-06-13T15:35:33Z

No description provided.

webapp/packages/core-data-context/src/DataContext/DataContext.ts

sergeyteleshev · 2024-06-14T10:59:38Z

webapp/packages/core-data-context/src/DataContext/DataContext.ts

-    version++;
-    this.map.set(context, value);
-    this.versions.set(context, version);
+    data.set(id, value);


in case where data was already defined it misses an if statement above where you set new context to the shallow observable store

this means that mobx wont be triggered if value changed in data, right? should we just move this line to the end of the function?

this.store.set(context, data);

no, here is all fine, to access data mobx will track both Maps and when we change value all subscribers to that value will be notified

webapp/packages/core-data-context/src/DataContext/DataContext.ts

webapp/packages/core-data-context/src/DataContext/useDataContext.ts

sergeyteleshev · 2024-06-14T11:27:36Z

webapp/packages/core-data-context/src/DataContext/useDataContextLink.ts

+export function useDataContextLink(context: IDataContext | undefined, update: (context: IDataContext, id: string) => void): void {
+  const [id] = useState(() => uuid());
+
+  useLayoutEffect(() => {


same for this one bad boy. I tried to remove both useLayoutEffects and at first glance the app works fine (did not invest many time to explore the whole app).

webapp/packages/core-data-context/src/DataContext/useDataContextLink.ts

devnaumov · 2024-06-14T14:04:41Z

webapp/packages/core-data-context/src/DataContext/DataContext.ts

+    this.store.clear();
+  }
+
+  private internalGet<T>(context: DataContextGetter<T>): T | typeof NOT_FOUND {


don't like this prefixes

devnaumov · 2024-06-14T14:08:03Z

webapp/packages/core-data-context/src/DataContext/DataContextGetter.ts

@@ -5,9 +5,11 @@
 * Licensed under the Apache License, Version 2.0.
 * you may not use this file except in compliance with the License.
 */
-import type { IDataContextProvider } from './IDataContextProvider';
+
+const typescriptTypeLink = Symbol('typescript type link');


Looks strange. Is there a way to avoid this?

I don't know any way to keep the generic type without using it in the object

webapp/packages/core-data-context/src/DataContext/useDataContext.ts

devnaumov · 2024-06-14T14:12:47Z

webapp/packages/core-data-context/src/DataContext/useDataContextLink.ts

+    }
+  });
+
+  useLayoutEffect(


Please fix lint errors

devnaumov · 2024-06-14T14:14:27Z

webapp/packages/core-navigation-tree/src/NodesManager/getNodesFromContext.ts

-  const node = context.tryGet(DATA_CONTEXT_NAV_NODE);
-  const getNodes = context.tryGet(DATA_CONTEXT_NAV_NODES);
+  const node = context.get(DATA_CONTEXT_NAV_NODE);
+  const getNodes = context.get(DATA_CONTEXT_NAV_NODES);


Seems like getNodes is always defined now

type of get changed from T to T | undefined and equal to type of tryGet. so it's basically tryGet replaced get

devnaumov · 2024-06-14T14:19:14Z

...gin-authentication-administration/src/Administration/Users/UsersTable/CreateUserBootstrap.ts

@@ -48,7 +48,7 @@ export class CreateUserBootstrap extends Bootstrap {
      },
      isDisabled: (context, action) => {
        if (action === ACTION_CREATE) {
-          const administrationItemRoute = context.tryGet(DATA_CONTEXT_ADMINISTRATION_ITEM_ROUTE);
+          const administrationItemRoute = context.get(DATA_CONTEXT_ADMINISTRATION_ITEM_ROUTE);


administrationItemRoute is defined now, not need for the ?.

devnaumov · 2024-06-14T14:20:06Z

webapp/packages/plugin-connections/src/ContextMenu/ConnectionMenuBootstrap.ts

@@ -68,13 +68,13 @@ export class ConnectionMenuBootstrap extends Bootstrap {
          return false;
        }

-        const connection = context.tryGet(DATA_CONTEXT_CONNECTION);
+        const connection = context.get(DATA_CONTEXT_CONNECTION);


Please check ?. here and all the way down where you change tryGet -> get

devnaumov · 2024-06-14T14:29:12Z

webapp/packages/plugin-user-profile/src/UserMenu/UserMenu.tsx

@@ -21,16 +22,19 @@ export const UserMenu = observer(function UserMenu() {
  const styles = useS(style);
  const authInfoService = useService(AuthInfoService);
  const menu = useMenu({ menu: MENU_USER_PROFILE });
+  const userInfo = authInfoService.userInfo;


Seems like this pattern is not stable as the developer can easily pass authInfoService.userInfo directly to the callback fn without reassigning this first

* CB-3510 feat: data editing actions based on view * CB-3510 fix: pr review * CB-3510 fix: table bottom bar styles

…a-context

Wroud added 3 commits June 13, 2024 17:58

CB-4018 refactor: data context api

37af6d9

CB-4018 refactor: data context api

841af0d

CB-4018 refactor: form parts api

25a745d

Wroud requested a review from devnaumov June 13, 2024 15:35

Wroud self-assigned this Jun 13, 2024

Wroud requested a review from sergeyteleshev June 13, 2024 15:35

sergeyteleshev requested changes Jun 14, 2024

View reviewed changes

CB-4018 fix: pr review

e8945b8

devnaumov requested changes Jun 14, 2024

View reviewed changes

sergeyteleshev self-requested a review June 14, 2024 14:35

sergeyteleshev approved these changes Jun 14, 2024

View reviewed changes

CB-4018 fix: track mobx dependencies in useDataContextLink

6bdbc57

devnaumov self-requested a review June 14, 2024 14:46

devnaumov approved these changes Jun 14, 2024

View reviewed changes

Wroud added 8 commits June 14, 2024 22:48

Merge branch 'devel' into refactor/cb-4018/data-context

cb6c651

Merge branch 'devel' into refactor/cb-4018/data-context

e666d91

CB-4018 fix: unsubscribe behaviour

81227ed

CB-3510 feat: data editing actions based on view (#2710)

8a39806

* CB-3510 feat: data editing actions based on view * CB-3510 fix: pr review * CB-3510 fix: table bottom bar styles

Merge remote-tracking branch 'origin/devel' into refactor/cb-4018/dat…

0a23993

…a-context

CB-4018 refactor: respect presentation in data viewer

ebd4c02

CB-4018 fix: remove gap in data viewer toolbar

64f89a0

CB-4018 fix: isolate contexts in data viewer

afb44cd

Wroud merged commit 9a471be into devel Jun 18, 2024
7 checks passed

Wroud deleted the refactor/cb-4018/data-context branch June 18, 2024 15:19

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor cb-4018 data context api #2708

Refactor cb-4018 data context api #2708

Wroud commented Jun 13, 2024

sergeyteleshev Jun 14, 2024

Wroud Jun 14, 2024 •

edited

Loading

sergeyteleshev Jun 14, 2024

devnaumov Jun 14, 2024

Wroud Jun 14, 2024

devnaumov Jun 14, 2024

Wroud Jun 14, 2024

devnaumov Jun 14, 2024

Wroud Jun 14, 2024

devnaumov Jun 14, 2024

Wroud Jun 14, 2024

devnaumov Jun 14, 2024

devnaumov Jun 14, 2024

devnaumov Jun 14, 2024

Wroud Jun 14, 2024

Refactor cb-4018 data context api #2708

Refactor cb-4018 data context api #2708

Conversation

Wroud commented Jun 13, 2024

Choose a reason for hiding this comment

Wroud Jun 14, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Wroud Jun 14, 2024 •

edited

Loading