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

Smallest reactive library #293

Open
anywhichway opened this issue Mar 27, 2024 · 3 comments
Open

Smallest reactive library #293

anywhichway opened this issue Mar 27, 2024 · 3 comments

Comments

@anywhichway
Copy link

anywhichway commented Mar 27, 2024

VanJS is not quite the smallest anymore, see https://github.com/anywhichway/trui. Although VanJS certainly rocks and it likely to get better support and be more pervasive. If there is any interest, I would be willing to take a go at making trui API compatible and explore reducing the size of the VanJS core with a contribution.

@Tao-VanJS
Copy link
Member

Tao-VanJS commented Mar 27, 2024

Thanks @anywhichway for sharing your library! This is definitely interesting work.

First, as of VanJS 1.5.0, the gzipped + minified bundle of VanJS is 1,055 bytes, not 1,704 bytes as mentioned in https://github.com/anywhichway/trui. I guess VanJS is still smaller :-)

Second, without a good document and a well-covered test suite, it's really hard to verify whether there are serious bugs in the code. Also without a good document, it's not even clear how the library is supposed to be used. For one thing, there isn't any garbage collection implemented in trui. I suspect there will be memory leaks in the scenarios described in this section.

Again, thank you for your work and sharing it!

@anywhichway
Copy link
Author

Odd, I get 3.19kb with terser and then down to 1704 with gzip. Are you calculating based on a module or non-module file?

Also oddly, if I take your latest minified file from public and gzip it I get 1384 characters, still larger than 1,055 bytes ... so I am not sure what to measure! Although I doubt I would ever match it anyway, so I adjusted the repo :-).

Granted I have not pounded on it yet, but I have not found any leaks using Chrome memory tracking. Primary references are stored in WeakMaps and secondary in regular Maps, these are continuously pruned. And yeah, test suite is not yet in place, which is why it has an "a" in the version.

The library is used pretty much the same way as VanJS, the examples in the doc are just about all based on VanJS examples.

I am sure you don't need any more noise in your repo. OK to close on your next reply.

@Tao-VanJS
Copy link
Member

I think it's perfectly ok to keep this thread open :-) These are interesting discussions. I think it's also beneficial to VanJS community to have relevant work discussed.

Odd, I get 3.19kb with terser and then down to 1704 with gzip. Are you calculating based on a module or non-module file?

For the minified file, van-1.5.0.min.js, it's 2035 bytes (the non-ESM counter part, van-1.5.0.nomodule.min.js is 2076 bytes). It is generated by terser with some specialized flags for further size optimization (see the cmdline).

Also oddly, if I take your latest minified file from public and gzip it I get 1384 characters, still larger than 1,055 bytes ... so I am not sure what to measure! Although I doubt I would ever match it anyway, so I adjusted the repo :-).

When van-1.5.0.min.js is gizpped, it will produce a 1055-byte .gz file. I am using gzip -kf <file path> (cmdline). Gzip version: Apple gzip 428, running in macOS 14.1.1 (Sonoma), Apple M1.

Regarding the memory leak, if I understand correctly, in trui, you're using dMap to manage the bindings of states. I only see new bindings being added to dMap, not seeing any of the bindings being cleaned up. For the example listed in this section, whenever the value of renderPre flips, the text state will be registered as a dependency of a new Text node. I am not seeing in your code that these registered bindings are being cleaned up anywhere, so eventually, there will be lots of bindings registered for text state, all except the last registered binding are obsolete. Eventually this will lead to lots of RAM allocated for nothing as the user interacts with the page more and more without reloading.

@anywhichway
Copy link
Author

Thanks for the continued dialog.

May have added these after you took your initial pass, not sure. May actually get some garbage from stranded observers.

    for(let d of [...set||[]]) {
        if(typeof d === "function") d(); // dependent is an observer not an element
        else if(!d.isConnected) set.delete(d);
        else if(d.isConnected && d!==this) rMap.get(d)?.call(d); // gets the render function for the dependent element
    }
if (this instanceof Node) {
    rMap.delete(this);
    this.replaceWith(e);
}

@Tao-VanJS
Copy link
Member

In the example listed in this section, the mechanism you proposed works when text state changes frequently. However, in a scenario where renderPre state changes far more frequently than text state, the bindings of text state can still be accumulated without getting a chance to be cleaned up as the code you propose will only be executed when text state changes.

Also, it's a bit more complicated to support garbage collection for derived states (observe in trui), which was introduced in VanJS 1.1.0. Otherwise, memory leaks can still occur for derived states for scenarios as the example listed in this section.

@anywhichway
Copy link
Author

anywhichway commented Mar 28, 2024 via email

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

2 participants