ENGWORK-5595: Make MVC compatible with React 18 #18
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
joshwilsonvu
commented
Oct 22, 2024
joshwilsonvu
commented
- ENGWORK-5595: Updated core logic in ApplicationView to be React 18 compatible. Started making ApplicationController compatible.
…mpatible. Started making ApplicationController compatible.
Comment on lines
+4
to
+6
| // TODO: It might be easier to replace this with `tsdx`, which makes it easy to build both ESM and | ||
| // CJS output with correct .d.ts files without worrying about externals. Using "exports" in | ||
| // package.json helps tools find the right files they need. |
There was a problem hiding this comment.
tsdx is awesome, I'd be happy to adopt that
Comment on lines
+143
to
+151
| // Same thing, except for class components, though we don't seem to be wrapping any class | ||
| // components in aha-app. Doesn't prevent tearing, since class components can't use | ||
| // `useSyncExternalStore`. The original plan was to render the class component under a function | ||
| // component similar to the above, only observing the class component's render method, but there's | ||
| // no way to reference the render method from the function component before rendering is complete. | ||
| // This is the best we can do. | ||
| // TODO: Actually, maybe we can pass a ref from the parent function component to the child class | ||
| // component and somehow use that to wire up the ViewStore with the class's `render()` method? Idk | ||
| // if that's possible. |
There was a problem hiding this comment.
I'm not sure if we need to support class components? The ones we have would date to pre-mvc I think, and if we were to adopt mvc we'd migrate them to function components.
There was a problem hiding this comment.
I agree—I just hate to do nothing and risk silent failures should we start wrapping class components. But maybe TS types that reject class components and documentation is enough.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.