Resource based vanilla renderer #99
Draft
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
|
|
NullVoxPopuli
changed the title
…pe checking to work (in both terminal (cd'd, and editor))
NullVoxPopuli
force-pushed
the
resourced-based-vanilla-renderer
branch
from
5e93b04
to
ee4d928
Compare
NullVoxPopuli
commented
Apr 22, 2023
| @@ -1,57 +1,77 @@ | |||
| /** | |||
| * TODO: | |||
There was a problem hiding this comment.
I added some todos from our convo the other day
NullVoxPopuli
commented
Apr 22, 2023
| } | ||
|
|
||
| export function Comment( | ||
| text: Reactive<string>, | ||
| description?: string | Description | ||
| ): ContentNode { | ||
| return ContentNode(({ into }) => { |
There was a problem hiding this comment.
I renamed this to just Content, because the type overload was breaking my brain, especially since the two ContentType's weren't exactly referring to the same function
NullVoxPopuli
commented
Apr 22, 2023
|
|
||
| RUNTIME.onFinalize(owner, cleanup); | ||
| RUNTIME.onFinalize(owner, () => void RUNTIME.finalize(formula)); |
There was a problem hiding this comment.
I don't know if this is right
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.
Atm, when rendering 1000 elements, using Resources here is almost twice as slow as the non-resource version.
The added test,
it can render many elements, (on my laptop) takes about 2.5s on themainbranch, and 3.8s on this branch.Until that is resolved, we probably can't merge this.
(Also, even before this PR, that's very slow -- will need to test again with a production build, but even in development 1000 elements is nothing)
Will need to do some analysis
Still WIP, I want to add more tests, and get confirmation on direction.
I also haven't been able to get anything related to
pnpm devrunning on this machine due tobut, idk, maybe the issue will go away in a floating dep update later 😅
This PR also adds
typescriptto both the vanilla and vanilla/tests package.jsons.This was required for running
pnpm test:typesin the vanilla and vanilla tests directories, as well as getting lsp support in my editor (I opened only to the vanilla folder, because I didn't want the rest of the repo adding to memory consumption of my machine (lsp, etc))