Skip to content

Considerations #7

@CodeSmith32

Description

@CodeSmith32

Hey @MananTank, sorry ahead of time! I would've put this in discussion, but discussion wasn't enabled.

This is more of a 'Wow, I just wrote almost this exact library myself AND thought to call it the same thing (which is how I found it)' moment. I then decided to review your code to see what I could learn!

For starters, thank you! Good to see how others have implemented this, and see if there's anything I can learn from it. (I really love the getter-setter bindings!)

A few things to consider / issues I've noticed with this approach:

1. Object prototypes are lost

From source: radioactive-state/blob/master/utils/reactify.js : 10

Your code generates an empty object or array, and then copies the properties over. This doesn't seem terrible at first, but there are a few concerns here:

  • It becomes problematic if a null prototype is intentionally used to prevent prototype pollution / __proto__ injection issues. (But this is normally only the case when using arbitrary key-value objects - see # 2.)
  • We cannot use objects like RegExp / Date in the state, because the methods disappear. (I see the workaround is to use .__target__, so, not really an issue.)

I found that using the actual object as the Proxy target seemed to work fine. In my project, when getting a property from a state proxy, if the value happens to be an object, I wrap it there in the getter; so initial recursive wrapping isn't necessary. This also saves on generating objects for unused sections of state. With that said, my approach does introduce an issue that allows creating non-reactive updates:

const array = [];

const Component = () => {
  const state = useRS({
    array,
  });

  const handler = () => {
    array.push("new value");

    // or

    const newArray = [];
    state.anotherArray = newArray;

    setTimeout(() => newArray.push(1, 2, 3), 1000);
  };
};

In your current code, array and state.array are different arrays (which might not be expected), whereas in my project they're the same, but changes to the unwrapped version don't react.

2. Special properties

In your code you have some (actually pretty neat!) special properties, __isRadioactive__, __target__, $, and any name that starts with $. I see this is useful in many cases, but has one major drawback: Using objects as arbitrary string key-value maps isn't really safe. Given the mention above about prototype pollution, these special properties become additional 'attack' targets. That is, assigning to any of these properties results in updating the target object, but reading any of them returns the special value associated with it. A better approach might be to use a WeakMap, and map the proxy objects to some metadata objects with these special values. Then, you can write functions that accept the proxy object, and return the value from this metadata, e.g.:

import { useRS, binding, mutations } from "radioactive-state";

const Component = () => {
  const state = useRS({
    property: "value",
  });

  const handler = () => {
    console.log(mutations(state));
  };

  return <input {...binding(state, "property")}/>
};

With that said, my guess is your library isn't intended to work with arbitrary key-value tables. If so, that's fine! The state.$property binding really does simplify auto-binding, and if the exception is not using arbitrary key-value tables, it may be worth the tradeoff.

3. Consider deep component trees

Radioactive state always works fine for a single component, and also works for nested components sometimes. But it does not work well if state is passed down to components wrapped with React's memo. This is because the proxy object itself doesn't change. Therefore, React decides that the child component doesn't need to re-render.

I took the sample todo app and wrapped the Todo component with memo (and then adjusted the Todos component to use stable callbacks). You can see that creating / removing todos still works, but checking them off stopped working:

https://stackblitz.com/edit/radioactive-state-issue-kpxdealv?file=src%2FTodos.jsx,src%2FTodo.jsx

This isn't a big issue - rather something to keep in mind. That is, reactivity simply doesn't work if memo is used for components that receive parts of the radioactive state. More specifically, all components in the tree that use any piece of the radioactive state must re-render when the parent component with the useRS hook re-renders.


None of these are major concerns. I just figured I'd share my experience. They seem more like they might instead be in conflict with the project's goals / design decisions. If so, feel free to close :)

Thanks!

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions