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

Replace RF component selector with new type #2491

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

joeyballentine
Copy link
Member

Follow up to #2490

Unfortunately, the RF node types aren't type-safe yet, but they will be in v12 (i think).

All this does is finish decoupling the RF node types from the node kinds, and replaces them with just a generic "backendNode" node type which uses our Node component.

@joeyballentine joeyballentine marked this pull request as draft January 21, 2024 19:14
@joeyballentine
Copy link
Member Author

This needs a migration... will do in a sec

@joeyballentine joeyballentine marked this pull request as ready for review January 21, 2024 19:18
@RunDevelopment
Copy link
Member

You just activated my trap card or something.

Anyway, we still have a bunch of code that uses node.type to get the node kind. You broke all of that, and since node.type is typed as string there aren't compiler errors. Fun. I would suggest using VSCode's "Find All References" to find all uses of .type.

I know this because I investigated this while doing #2490, but I didn't make this change in this PR, because it seemed like it should be its own PR. Well, I guess that PR is right now :)

@joeyballentine
Copy link
Member Author

Hmm, I think I'll just close this and wait til we update to RF v12 to make this change, so that it will actually be typed correctly

@RunDevelopment
Copy link
Member

You can also keep it open. I'm gonna work on #2266 tomorrow and I'll change most of the code using node.type. So you'll only have to fix maybe one or two occurrences after that.

@joeyballentine joeyballentine marked this pull request as draft January 26, 2024 01:48
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

Successfully merging this pull request may close these issues.

2 participants