Skip to content

is it a good idea to use __SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED? #86

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

Closed
igorkamyshev opened this issue Sep 7, 2022 · 8 comments

Comments

@igorkamyshev
Copy link

As I understand, React bindings of this package are using __SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED variable from react package.

It looks a little bit dangerous, doesn't it? It looks like it could break after any minor update of React.

Monkey patching of React internals looks are even more dangerous for me.

Is React bindings of suitable for production usage?

@Rel1cx
Copy link

Rel1cx commented Sep 7, 2022

"its-fine"

@trhinehart-godaddy
Copy link

Example using signals-core in React without signals-react hijacking internals via a useSignal util

https://codepen.io/gingur/pen/gOzaywx

@trhinehart-godaddy
Copy link

trhinehart-godaddy commented Sep 7, 2022

Can also use an HOC if you want to use signals in a Component Class

function connectSignal(obj) {
  const entries = Object.entries(obj);
  return (Component) => hoist((props) => {
    const connectedProps = Object.fromEntries(entries.map(([key, value]) => [key, useSignal(value)]));
    return <Component { ...props } { ...connectedProps }/>
  }, Component);
}

https://codepen.io/gingur/pen/NWMGVdZ

calebgregory added a commit to calebgregory/sweet-petit-feastival-app that referenced this issue Sep 12, 2022
@marvinhagemeister
Copy link
Member

Closing this issue because with #219 the code doesn't use everyone's favorite internal property anymore 🎉

@rollie42
Copy link

rollie42 commented Nov 1, 2023

It was added back in on Mar 30, '23 here from @andrewiggins as part of some reworking/optimizing, which was subsequently moved and documented here. It sounds like there are good justifications for omitting the solution referenced above, but the discussion goes a bit over my head.

It feels like this issue should be reopened until a performant way of achieving the same goal is found or made available officially by react team - I stumbled onto this thread after reading up on signals and finding company slack discussions between colleagues where their use was not advised specifically because of the perceived risk in importing internal tooling with such ominous naming, and no doubt others have similar misgivings.

@ADTC
Copy link

ADTC commented Nov 4, 2023

Yeah, was super excited to use Signals, but I don't think my boss would approve it for our project, with a name like __SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED.

Obviously, I don't want to be 🔥d...

@swar30
Copy link

swar30 commented Apr 8, 2024

@andrewiggins if you can please clarify this topic, I see that as part of 6717601#diff-b36af6e1b379a3a89ec07f13fdb433bd3a8a8fa64cc8a7a7efbc20c69380efebR4 usage of __SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED was re-inroduced, or moved from somewhere else in code.

where does this issue stand, are there plans to move away from using __SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED ?

@XantreDev
Copy link
Contributor

@andrewiggins if you can please clarify this topic, I see that as part of 6717601#diff-b36af6e1b379a3a89ec07f13fdb433bd3a8a8fa64cc8a7a7efbc20c69380efebR4 usage of __SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED was re-inroduced, or moved from somewhere else in code.

where does this issue stand, are there plans to move away from using __SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED ?

For now it's an opt-in option for backwards compatibility. You need to use the babel plugin with a new approach

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants