sveltejs / svelte

Cybernetically enhanced web apps
https://svelte.dev
MIT License
78.04k stars 4.08k forks source link

Provide more control over what gets proxified, pontentially making it opt-in #12364

Closed danielo515 closed 3 days ago

danielo515 commented 1 month ago

Describe the problem

I have an Obsidian plugin using Svelte 4. Being a plugin in a non conventional web environment means a lot of interaction between regular JS and svelte components. This is being a problem because every time data goes through a svelte component everything gets proxified. I don't even migrated all my components and, as soon as some data crosses the Svelte boundaries I get a deeply proxified object back. This is very challenging, first because I don't know what the proxy application rules are, and secondly because, even if I do, I will have to take a lot of care to unproxify stuff. I'm already getting a lot of trouble because of this: stores that get values that are proxies, infinite reactivity loops and other annoyances that are preventing me from migrating to Svelte5

Describe the proposed solution

I want to make proxies opt-in, or better document when and why things are converted to proxies. I don't think such agresive level of proxification is necessary. Svelte 4 had a better opt-in mechanism where you just use $: blocks to tell the compiler to track blocks and other stuff.

Importance

i cannot use svelte without it

paoloricciuti commented 1 month ago

Proxies is already kinda opt-in if you use $state.frozen...can you provide a reproduction in the repl of what your use case is?

danielo515 commented 1 month ago

Do you have any template that uses svelte 5 that is not the REPL?

El mar, 9 jul 2024, 18:02, Paolo Ricciuti @.***> escribió:

Proxies is already kinda opt-in if you use $state.frozen...can you provide a reproduction in the repl of what your use case is?

— Reply to this email directly, view it on GitHub https://github.com/sveltejs/svelte/issues/12364#issuecomment-2218089897, or unsubscribe https://github.com/notifications/unsubscribe-auth/AARKJWOOOSJJFZIWK6R34HDZLQCRZAVCNFSM6AAAAABKTFI54OVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDEMJYGA4DSOBZG4 . You are receiving this because you authored the thread.Message ID: @.***>

paoloricciuti commented 1 month ago

Do you have any template that uses svelte 5 that is not the REPL?

What do you mean?

rChaoz commented 1 month ago

I think $state.frozen provides additional limitations that might not work for everyone. Sample Svelte 4 code that can't be converted:

let x = {}
x.prop = 123  // reactive update

// in a listener
doSomething(x)  // 3rd party func, can't handle proxies

IMO the best solution and related issue: #10560

paoloricciuti commented 1 month ago

I think $state.frozen provides additional limitations that might not work for everyone. Sample Svelte 4 code that can't be converted:

let x = {}
x.prop = 123  // reactive update

// in a listener
doSomething(x)  // 3rd party func, can't handle proxies

IMO the best solution and related issue: #10560

let x = $state({});
x.prop = 123; // reactive update

// in a listener
doSomething($state.snapshot(x));
rChaoz commented 1 month ago

@paoloricciuti I understand that's a solution, but when state gets sent around various functions/files it's hard to keep track of what is a proxied state and what's not, especially that reactivity now crosses function boundaries, so you generally want it to do that. As the original author said:

I will have to take a lot of care to unproxify stuff.

I know that it's "doable" to just deal with the proxies, it's not ideal for every use case, though. While $state.snapshot is generally great, using it for states you always share with 3rd party code makes it feels like a solution to a to the very problem it creates - that you can't have un-proxied state. Which is also why $state.is even exists in the first place, and it's also why issues on this theme keep appearing, such as #12128.

paoloricciuti commented 1 month ago

@paoloricciuti I understand that's a solution, but when state gets sent around various functions/files it's hard to keep track of what is a proxied state and what's not, especially that reactivity now crosses function boundaries, so you generally want it to do that. As the original author said:

I will have to take a lot of care to unproxify stuff.

I know that it's "doable" to just deal with the proxies, it's not ideal for every use case, though. While $state.snapshot is generally great, using it for states you always share with 3rd party code makes it feels like a solution to a to the very problem it creates - that you can't have un-proxied state. Which is also why $state.is even exists in the first place.

This is why I'm asking for a more concrete how many libraries that doesn't accept proxy you have to move your state to?

Your example is great I understand the problem better now but I still struggle to see how that is a huge problem that should basically revert the current progress of svelte 5 (since is basically not possible to have deeply nested reactivity that also works when you add properties without a proxy)

danielo515 commented 1 month ago

I can try to provide a concrete example but "explaining" it, because, as I said, my environment is quite particular to create a reproduction in the sparse time I have. In any case, if anyone is interested, here is my own PR to migrate (attempt) to svelte 5: https://github.com/danielo515/obsidian-modal-form/pull/300/files

First, the problem is not with $state rune, that I can control, the problem is that, everything getting into a component props is turned into a proxy. My obsidian plugin allows to define forms using a JSON DSL, and then open those forms in modals. Right now, there are two ways to open a modal:

One may expect identical behavior, since all the preview does is call the API:

// Editor code
    const handlePreview = () => {
        if (!isValidFormDefinition(definition)) return;
        onPreview(definition);
    };
// ...
<button type="button" on:click={handlePreview} disabled={!isValid}>
Preview
</button>

This is the component instantiation:

        this.formEditor = createClassComponent({
            component: FormEditor,
            target: this.containerEl,
            props: {
                definition: this.formState,
                app: this.app,
                onChange: () => {/* ... */},
                onSubmit: (formDefinition: FormDefinition) => { /* ... */},
                onCancel: () => {
                    this.plugin.closeEditForm();
                },
                onPreview: async (formDefinition: FormDefinition) => {
                    const result = await this.plugin.api.openForm(formDefinition);
                    const result_str = JSON.stringify(result, null, 2);
                    log_notice("Form result", result_str);
                    console.log(result_str);
                },
            },
        });

However, calling the API from any javascript works as expected, but calling it through the FormEditor component results in an infinite reactivity loop because the formDefinition has been turned into a proxy, which is fed to several stores that, I'm not sure yet how, are getting triggered by that proxy and the proxy is somehow being triggered by the stores (I suspect because deeply nested reactivity). Please note that inside the FormEditor component I'm not using at any point $state(formDefinition) or anything like that, it is being transformed into a proxy just because I'm feeding it to the component. It was very hard, first to identify the root cause of the problem (some random thing becoming a nested proxy) and also, was not obvious to find the trace or when or why it was happening, specially because: JS -> API.open -> Svelte(OK)
but JS -> Svelte -> API.open -> Svelte (infinite loop)

There was no type indication that the values were being transformed, and until I debugged step by step the runtime code, I had no idea of what was happening. I can only imagine a lot of situations in the future where this kind of subtle bugs are very hard to track, just because you forgot to 'un-proxy' an object that crossed a Svelte component. This will make integrating svelte components into third party projects a lot harder, something it was not like that before. Svelte (correct me if I'm wrong) started as a way to create independent UI widgets, but this situation is kind of moving it to be more like a walled garden framework, where everything needs to live within Svelte land and were you will need to make sure to do the appropriate conversion at the boundaries.

This also raises to me the question of, who is the responsible of removing the proxy? The svelte component? As you saw in my example, it has no idea if it is being called from normal JS code or from another Svelte component, or even something in between. But if we say the responsible of doing the conversion is the calling code, then you will need svelte compiler as a dependency, and then rename your otherwise perfectly normal file to whatever.svelte.ts just to be able to properly use svelte components. Both approaches feel wrong to me, and really break the encapsulation capabilities of the framework. Proxies seems like a leaky abstraction, and I think I should not be taking care of them

7nik commented 1 month ago

I can try to provide a concrete example but "explaining" it, because, as I said, my environment is quite particular to create a reproduction in the sparse time I have. In any case, if anyone is interested, here is my own PR to migrate (attempt) to svelte 5: https://github.com/danielo515/obsidian-modal-form/pull/300/files

It would be nice to provide reproduction steps or code because it's not enough to clone the repo and do npm run dev. I'd look into it, but I don't want to spend an uncertain amount of time just to figure out how to run the plugin.

danielo515 commented 1 month ago

It would be nice to provide reproduction steps or code because it's not enough to clone the repo and do npm run dev. I'd look into it, but I don't want to spend an uncertain amount of time just to figure out how to run the plugin.

I understand your point of view, and I agree, is hard to understand a problem you can not debug. Do you use Obsidian? If so, I can guide you to run the plugin. If not, I can try to create a reproduction, but it seems very time consuming without a template

paoloricciuti commented 1 month ago

but it seems very time consuming without a template

The point is that if the problem is with svelte you should be able to reproduce it with svelte only. And I know is time consuming. Usually I spent 2 or 3 hours to come up with a good reproduction for unintuitive bugs, sometimes I don't even submit the bug because I can't properly reproduce.

The point is that if you don't spend this time on the reproduction the maintainer or contributor that is working on this will need to do it anyway, you are just shifting the burned to someone else which is already giving you a library to save you time.

danielo515 commented 1 month ago

I am not asking anyone to do the work for me, I'm kindly asking if anyone has a project already setup with svelte 5, so I can focus on the reproduction rather than configuring typescript, a bundler, and a minimum app shell

paoloricciuti commented 1 month ago

I am not asking anyone to do the work for me, I'm kindly asking if anyone has a project already setup with svelte 5, so I can focus on the reproduction rather than configuring typescript, a bundler, and a minimum app shell

pnpm create svelte and select svelte 5 as an option :)

Or if you prefer https://sveltelab.dev/?t=next

dummdidumm commented 1 month ago

Can you try with the latest version of Svelte 5? createClassComponent used proxy under the hood until recently, but doesn't anymore now, so when you're not using $state you shouldn't get an unwanted proxy. Alternatively you can avoid createClassComponent and use mount instead, which sidesteps this completely.

danielo515 commented 1 month ago

That sounds promising, I will be testing that. Just to ble clear, I was not using $state at all, the component was using the old syntax, it is not in runes mode.

On Mon, Jul 15, 2024 at 10:11 AM Simon H @.***> wrote:

Can you try with the latest version of Svelte 5? createClassComponent used proxy under the hood until recently, but doesn't anymore now, so when you're not using $state you shouldn't get an unwanted proxy.

— Reply to this email directly, view it on GitHub https://github.com/sveltejs/svelte/issues/12364#issuecomment-2227926198, or unsubscribe https://github.com/notifications/unsubscribe-auth/AARKJWJPRYKP6L6YMFCFFDDZMN73DAVCNFSM6AAAAABKTFI54OVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDEMRXHEZDMMJZHA . You are receiving this because you authored the thread.Message ID: @.***>

--

https://danielorodriguez.com

danielo515 commented 1 month ago

@dummdidumm the latest svelte release indeed does not trigger my infinite loop (just like svelte 4 doesn't either). This demonstrates that my initial judgment was right, and the problem was indeed the proxy triggering the store reactivity that was in turn mutating the proxy. I know that I did not provided a reproduction yet (sorry I'm very busy this days) but I don't think it is that important. The point is that "proxies everywhere for everything without user control" is not that good of an idea, and that giving more control around what and when is turned into proxies is something desirable. Also, it may be worth pointing out in the docs or somewhere that proxies and stores are not a good idea to mix. This also makes me worry if the only viable reactivity solution that will work fine with svelte going forward is going to be runes, where everything else will be problematic to integrate.