Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Migrate off backbone.js, jQuery and underscore.js #4286

Open
maxpatiiuk opened this issue Dec 14, 2023 · 5 comments
Open

Migrate off backbone.js, jQuery and underscore.js #4286

maxpatiiuk opened this issue Dec 14, 2023 · 5 comments
Assignees
Labels
1 - Enhancement Improvements or extensions to existing behavior

Comments

@maxpatiiuk
Copy link
Member

maxpatiiuk commented Dec 14, 2023

We talked about this a lot, but didn't have an explicit issue for this.

Backbone.js is used for representing database resources on the front-end. That library also uses jQuery and underscore.js. That's the only place where jQuery and underscore.js are still used.

Backbone.js is not very compatible with the way react works (it's not reactive at all - making relatively simple things very complicated due to need for manually setting up listeners in useEffect - see #4264 for an example), is not very type-safe (may return null or undefined, rather than always undefined)

Closely related to #2621 (and see #2621 (comment))

This is going to be a somewhat large project because:

  • Resources are being used in a lot of places
  • Data integrity is the most important thing. Need to be sure not to introduce bugs. Need to write good tests
  • There is also a concept of SerializedResource that was a temporary stop-gap measure and should be merged with the final solution

We could write our own library for representing resources on the front-end, but it would be best to do at least some research into available options before doing so (surely some new nice thing has became available in the last 15 years since backbone.js)

@maxpatiiuk maxpatiiuk added the 1 - Request A request made by a member of the community label Dec 14, 2023
@github-project-automation github-project-automation bot moved this to 📋 Backlog in Front-End Backlog Dec 14, 2023
@maxpatiiuk
Copy link
Member Author

maxpatiiuk commented Dec 15, 2023

What I am dreaming of:

  1. A central store that holds all fetched objects in the front-end
  2. React components receive immutable references to those objects (either table name + id, or table name with temporary guid for new objects*)

then, inside the component, it can call a hook like useResourceField or useResource, that would in turn resolve a single field value from the object, or entire object - and as a result, re-render on that field change, or any change
in non-reactive code (business rules), you would still be able to resolve the reference to an object to the mutable object itself

*so the above can be implemented even with backbone remaining in place (in fact, we have a useResource that kind of does that already), but there are other issues with backbone, so bandaids like this are not the best long-term solution

*on further thought, probably all references should be table name + front-end only GUID, so that if new resource becomes saved, the previous reference still holds true

At the same time, I would ideally like to have a separate typescript type for saved resource (or reference to a resource), new resource, or a type that accepts both - this would add type safety and would clearly show whether react component expects to receive a saved resource, or if it doesn't care either way

Additional benefit of central store for all objects is the fact that we can load the same object again in the code, and not worry about caching as load would get the object from the central store
also means that any changes to the object would be centralized between all the components that use that object, avoiding out of sync bugs AND worse, potential data loss when there are two instances of the same database object

So now just to find a library that can do the above, or as close as we can, or else implement our own

@maxpatiiuk maxpatiiuk added this to the Max's bucket list milestone Dec 15, 2023
@maxpatiiuk
Copy link
Member Author

maxpatiiuk commented Dec 17, 2023

Some suggested libraries after playing with ChatGPT with AutoExpert plugin:

For state management:

  • Redux Toolkit: Offers good TypeScript support, integrates well with React, and simplifies Redux usage.
  • Recoil: A state management library by Facebook, it provides more granular re-render control and might be simpler to integrate with React compared to Redux.
  • Zustand: A minimalistic state management solution that can be a good fit if you prefer a simpler approach than Redux.
  • SWR or React Query: For data fetching, caching, and synchronization, these libraries can efficiently manage server state in React applications.

For ORM aspects:

  • MobX-State-Tree (MST): MST works well with MobX and provides a more structured approach to state management. It offers features like type-checking, snapshots, and reactive state that aligns with your requirements. It's suitable for managing complex application states and has good TypeScript support.
  • Redux-ORM: This is a Redux-specific library that provides ORM-like capabilities. It's great for managing normalized state, relationships, and complex updates. Its integration with Redux Toolkit can ensure seamless TypeScript support and easy integration with React.
  • WatermelonDB: Designed to be highly scalable, WatermelonDB offers advanced database-like features, including lazy loading and observable query results, making it suitable for applications with complex data structures. While it's more database-focused, it can be adapted for REST API interactions.
  • Normalizr: This library helps to normalize nested JSON according to a schema. It's not an ORM but is useful for managing complex data structures received from APIs. It pairs well with state management libraries like Redux or MobX.
  • React Query or SWR: While not ORMs, both libraries are excellent for data fetching, caching, and synchronization in React applications. They can reduce the need for complex state management and provide hooks for data manipulation.
  • TypeORM: Primarily a back-end ORM for TypeScript, TypeORM can be adapted for front-end use. It offers advanced ORM features, but its use in front-end scenarios might require a more custom setup.
  • mikro-orm.io
  • Sequelize

TODOs:

  • Explore whether a more structured state library (like Redux/MobX) would provide benefits over the more ad-hock approach we are using right now (many small independent states)
    • If so, toolkits built on top of these libraries that add ORM features would be worth exploring
  • Investigate React Query and SWR as they can benefit all places where network requests are made, not just ORM

@maxpatiiuk
Copy link
Member Author

Exploring ref as a string

A ref should probably be just front-end only GUID, nothing else. this way it's a string, thus preventing a bug where a new instance of an object is created on each render (as could happen with a ref like {readonly tableName:keyof Tables; readonly guid: string})

Then, the following types could be used for the GUID (and from now on, I will refer to GUID as Ref):

export type NewResourceRef<TABLE extends keyof Tables> = Nominal<string, `NewResourceRef_${TABLE}`>;
export type SavedResourceRef<TABLE extends keyof Tables> = Nominal<string, `SavedResourceRef_${TABLE}`>;
export type ResourceRef<TABLE extends keyof Tables> = NewResourceRef<TABLE> | SavedResourceRef<TABLE>;

// now, in the code can use these types as argument types in functions/components or in return types:
NewResourceRef<'Accession'>  // ref to new Accession resource
SavedRecordRef<'CollectionObject' | 'Accession'>  // ref to saved CollectionObject or Accession resource
ResourceRef<keyof Tables & `${string}Attachment`>  // ref to a saved or new resource, from any table whose name ends with Attachment
ResourceRef<keyof Tables>  // a ref to any resource from any table

// see more about nominal types - https://zackoverflow.dev/writing/nominal-and-refinement-types-typescript
const Brand = Symbol();
export type Nominal<Type, Name extends string> = Type & {
  readonly [Brand]: [Name];
};

The above Nomintal type is very simple (copied from https://www.npmjs.com/package/nominal-types)
But there is also this library that provides more features - https://www.npmjs.com/package/@coderspirit/nominal (TODO: explore it)


Then, the following utils would be available:

// resolve ref:
const getResource = <TABLE extends keyof Tables>(ref: ResourceRef<>): Resource<TABLE> => {
  return resourceStore[]
};
// a hook that resolves ref and reactively updates on object changes
const useResource = <TABLE extends keyof Tables>(): Resource<TABLE> => {};

(using Resource rather than shorter Record as Record conflicts with TypeScript's native Record type)

That all is cool, but:
since we are looking to have a central store of all fetched objects, we should use WeakMap to avoid memory leaks - and WeakMap requires object keys, thus necessitating object refs.

Exploring Object refs

We could have refs be just {} - simple and memory efficient.
However, having some sort of guid in the ref would be helpful as it can be used as a key={} prop when looping over objects in react
Additionally, might want to have table name and id in the ref for convenience (without it would need something like getResource(ref).id and getResource(ref).tableName

Also, "guid" may be confused with the "guid" database field, so from now on, I would call it uuid instead.

type NewResourceRef<TABLE_NAME extends keyof Tables> = {
  readonly table: Tables[TABLE_NAME];
  readonly uuid: number;
  readonly id: undefined;
};
type SavedResourceRef<TABLE_NAME extends keyof Tables> = {
  readonly table: Tables[TABLE_NAME];
  readonly uuid: number;
  readonly id: number;
};
type ResourceRef<TABLE_NAME extends keyof Tables> = NewResourceRef<TABLE_NAME> | SavedResourceRef<TABLE_NAME>;

type NewResource<TABLE_NAME extends keyof Tables> = NewResourceRef<TABLE_NAME> & {
  data: Record<string, unknown>;
  readonly reference: NewResourceRef<TABLE_NAME>;
};
type SavedResource<TABLE_NAME extends keyof Tables> = SavedResourceRef<TABLE_NAME> & {
  data: Record<string, unknown>;
  readonly reference: SavedResourceRef<TABLE_NAME>;
};
type Resource<TABLE_NAME extends keyof Tables> = NewResource<TABLE_NAME> | SavedResource<TABLE_NAME>;

const store = new WeakMap<ResourceRef<keyof Tables>, Resource<keyof Tables>>();
const tables = {} as { readonly [TABLE_NAME in keyof Tables]: Tables[TABLE_NAME]; };
const genericTables = {} as Readonly<Record<keyof Tables,Tables[keyof Tables]>>;

const makeUuid = ()=>Math.random();
async function fetchResource<TABLE_NAME extends keyof Tables>(tableName:TABLE_NAME, id:number): Promise<Resource<TABLE_NAME>> {
  const reference: SavedResourceRef<TABLE_NAME> = { table: tables[tableName], id, uuid:makeUuid() };
  // simplified:
  const resourceData = await fetch('').then((response)=>response.json());
  const resource = {...reference, reference, data: resourceData as Record<string,unknown> };
  store.set(reference, resource);
  return resource;
}

still need to figure out how "new resource" gets converted into "saved resource" when it's saved (how to do so type safely and bug-free)
i.e references are supposed to be immutable, but when resource changes, the id in the reference changes from undefined to number - so have "id" not be readonly in the reference?
or if reference is immutable, then we need to create a new reference to the resource once the resource is saved
but if there is ever more than one reference to a resource, then we face a problem of having multiple references to the same resource

other questions:

  • do we actually care about having different types for new vs existing resource? in which places would that increase type safety? can't we just have id type be undefined | number always?
  • if resource reference is an object, why do we need reference at all rather than passing the object directly (like we do now)?

here I am realizing something: given that a WeakMap key must be an object, and we want to ensure there is only one instance of a given object at a time, then there is no way to resolve a tableName+id to an existing WeakMap entry, because any mapping between tableName+id to resource or resource ref would prevent garbage collection.
thus, we can have one of these 3 approaches, but no all at once:

  1. no garbage collection for objects. can use string or objects as references
  2. we implement manual garbage collection (I know, a really big can of worms), can use string or objects as references. example implementation of garbage collection: if any useResource/useResourceValue hook is referring to an object, than the object is used, if no hook is referring to an object, then remove the object from the store after x seconds (i.e 60 seconds). The 60sec delay accounts for object being using in business rules/other non react code, after which it's either no longer needed (thus can be removed from the central store), or now used by some component in the useResource hook. This of course is not full-proof - besides the added complexity, in some cases it would result in object being removed from central store while it still is being used (for example, the Collection object is not directly used by any react component, but is indirectly needed several times), and thus potentially causing duplicate instance of the same database object if some later place wants to fetch it again. There are many complications we could add to make it smarter (like exclute tables like Insitution, Collection, TaxonTreeDef and etc from garbage collection), but that all adds complication and possibility for bugs
  3. garbage collection. no central store (thus no caching of fetched resources and no ensuring that only a single instance of a resource exists) - this is essentially what we have right now
  4. do caching on the browser level (cache results of resource fetch network requests) - bad idea as this way we have no idea if resource has been modified or deleted - could have cache be short lived (like 60s or 10s) - this reduces likelihood of stale cache, but also dramatically reduces the usefulness of the cache in the first place

The reason I am a bit afraid of option 1 is that in cases of query results, or attachments viewer, thousands of objects can be created - no garbage collection could mean big memory leaks
Option 4 is just bugs waiting to happen (and not just any bugs, but really hard to debug bugs)
Option 3 is not appealing as we are not getting any benefits over what we have now
Option 2 brings a lot of complexity and possibility of bugs - though, the possible bugs with this approach are the same kind of bugs we could have right now (two instances of the same object, or object fetched twice needlessly), so in the worst case it's not worth than 3 - and the added complexity could be mitigated with added tests (which are a must anyway).

TODOs:

@maxpatiiuk
Copy link
Member Author

maxpatiiuk commented Jan 1, 2024

I have just remembered that JavaScript recently gained a new feature - WeakRef, which is exactly what we need - a reference to the object, that does not prevent garbage-collection.

Then, for an object ref, I decided to go with this shape:

{ tableName: keyof Tables, id: number }
{ tableName: keyof Tables, id: string }

where id is number for saved resource, but a string UUID for an unsaved resource. benefits:

  • you can always use id as a resource identifier (i.e as a React key prop), regardless of whether resource is saved or not
  • when you need to resolve a resource ref, table name and id are sufficient for that - no need for UUID - thus, one less argument to pass to functions

you can check if resource is saved using typeof ref.id === 'number'

Experimental implementation on the new-resource-api branch.
Still need to resolve some typing weirdnesses.
Don't like having event-based reactivity, so will look at libraries mentioned previously for better solutions

See #4339 for work in progress code

TODO: in tests, save the value of the store in beforeAll hook, and restore the value in afterAll hook (basically, global context objects should be preserved, but all others should not be saved to the context). or maybe simply disable the global context when in tests?

@sharadsw
Copy link
Contributor

sharadsw commented Sep 6, 2024

From #5139:
A new resourceApi would require more flexible fetching options and more control over network requests in general so that we can avoid using workarounds like hijackBackbone()

@grantfitzsimmons grantfitzsimmons added 1 - Enhancement Improvements or extensions to existing behavior and removed 1 - Request A request made by a member of the community labels Dec 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1 - Enhancement Improvements or extensions to existing behavior
Projects
Status: Unsorted
Status: 📋 Backlog
Development

No branches or pull requests

4 participants