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

Svelte 5: Simplify client-side API #9827

Closed
Rich-Harris opened this issue Dec 7, 2023 · 11 comments · Fixed by #10516
Closed

Svelte 5: Simplify client-side API #9827

Rich-Harris opened this issue Dec 7, 2023 · 11 comments · Fixed by #10516
Milestone

Comments

@Rich-Harris
Copy link
Member

Describe the problem

At the moment we have a pair of mildly confusing APIs for creating components — createRoot and mount. If I'm completely honest I can't remember what the difference is and why we have both. I'm sure there's a good reason, I just don't care. It's too complicated.

One thing I do know is that the createRoot function adds $set and $destroy methods, similar to the ones we have on legacy classes. This means that you can do this sort of thing:

const app = createRoot(App, {
  target,
  props: { count: 0 }
});

app.count += 1; // huh?
app.$set({ count: app.count + 1 }); // huh?
app.$destroy();

Honestly, it's all a bit weird and ugly. On top of that, the app.count getter/setter only exists if you compile with the accessors: true option, which causes a bunch of extra code to be generated.

Describe the proposed solution

Let's do this:

import { createProps, mount } from 'svelte';
import App from './App.svelte';

const props = createProps({ count: 0 });
const unmount = mount(App, { target, props });

// later
props.count += 1;
unmount();

Or, for components where you don't need to update the props from outside after the component has been created:

import { mount } from 'svelte';
import App from './App.svelte';

const unmount = mount(App, { target, props: { count: 0 } });

// later
unmount();

Here, createProps is just a wrapper around the internal proxy function (or maybe just a re-export, for that matter?) There's no createRoot, no $set, and no $destroy (except for the return value from mount, mirroring the $effect.root(...) signature. Props behave exactly like they do in runes mode. accessors is unnecessary, and therefore deprecated.

Alternatively, keep the createRoot name and ditch mount

Alternatives considered

n/a

Importance

nice to have

@Rich-Harris
Copy link
Member Author

(Bonus: if we do this, proxy no longer needs the immutable second argument)

@dummdidumm
Copy link
Member

dummdidumm commented Dec 7, 2023

IIRC you brought createProps up a while ago, and we decided against it. I'm still against it, it just feels weird to indirectly update the component like this (feels dirty; it doesn't simplify the API, it complicates it because I now need two things to interact with the component). I vote removing createRoot and making mount the only thing you have, but with the API of createRoot (i.e. return $destroy and $set). The fact that you can modify props through accessors is for backwards compatibility, and doesn't affect the weirdness or un-weirdness of the API. If you imagine a world without accessors, mount with $set and $destroy makes perfect sense to me.

The reason why we introduced createRoot was to save a few bytes. SvelteKit for example would only need mount because it doesn't need to imperatively interact with the component afterwards. This is the one argument in favor of createProps, because it would mean that theoretically people can get around the additional code that comes with the proxy, if they don't need that behavior and don't use proxies anywhere else in their app. But I'm not sure if that is actually happening in practise, so it's likely irrelevant.

I don't fully understand why having createProps would remove the need for the immutable argument.

@trueadm
Copy link
Contributor

trueadm commented Dec 7, 2023

We definitely don't need createProps, it doesn't even make sense as props are now read-only. What if you passed in reactive state to the root and could mutate that object instead?

I actually think we should have two APIs:

  • mount, works like today, and without createRoot and createProps.
  • legacyMount, works like createRoot but has the $destroy and $set APIs. This should be deprecated.

I don't think you should be able to set props on a mounted component. It's just not needed and feels very non-declarative.

If you want to change what gets passed in, you just change the state directly:

const props = $state({ });

mount(App, { props, target: document.body });

props.foo = 'something else';

@dummdidumm
Copy link
Member

Imperative control over the component is important for testing, so we shouldn't remove it IMO. How else would you test that the component does what you want?

@trueadm
Copy link
Contributor

trueadm commented Dec 7, 2023

@dummdidumm Simple:

const props = $state({ });

mount(App, { props, target: document.body });

@dummdidumm
Copy link
Member

dummdidumm commented Dec 7, 2023

That would require people to also preprocess their test files before passing them to whatever - is that doable? cc @dominikg

What if you're in an environment where you only interact with / consume the end result, and don't want to add the Svelte compiler to your tool chain?

@Rich-Harris
Copy link
Member Author

Couple more thoughts:

  • it might be worth making mount and hydrate async. that way, we have the option of (for example) making hydration non-blocking in future without it being a breaking change. the alternative is to later introduce mountAsync and hydrateAsync but that's ugly, especially if we also wanted to deprecate the sync versions
  • component exports need to be represented in the return value somewhere
  • I quite like the const props = $state({}) idea here, it makes a ton of sense. In situations where you need to create state in tests, you could do that easily enough by creating your own createProps function:
// tests/test.ts
import { mount, flush } from 'svelte';
import { it, assert } from 'vitest';
import { createProps } from './utils.svelte.js';
import Component from './Component.svelte';
 
it('works', async () => {
  const props = createProps({ name: 'world' });
  const target = document.createElement('div');
  const [unmount, exports] = await mount(App, { props, target });
  
  assert.equal(target.innerHTML, '<h1>hello world!</h1>');

  props.name = 'everybody';
  flush();

  assert.equal(target.innerHTML, '<h1>hello everybody!</h1>');

  unmount();
});
// tests/utils.svelte.js
export function createProps(data) {
  const props = $state(data);
  return props;
}

@trueadm
Copy link
Contributor

trueadm commented Dec 7, 2023

@Rich-Harris We don't have a hydrate API, are you suggesting we add one?

As for making them async – this makes sense to me.

@Rich-Harris
Copy link
Member Author

Sorry, that was confusing — that was partly a follow-up to a whole separate conversation in Discord, about making the hydration library code treeshakeable

@SomaticIT
Copy link
Contributor

One thing against the let props = $state({}) is that you can't use it outside svelte. One promise of Svelte is that it generates vanilla JS. You can use your component in a non-svelte environment. The old way ($set/$destroy) makes it easy to use your components in any environment.

@dominikg
Copy link
Member

That would require people to also preprocess their test files before passing them to whatever - is that doable? cc @dominikg

What if you're in an environment where you only interact with / consume the end result, and don't want to add the Svelte compiler to your tool chain?

@dummdidumm what exactly do you want to preprocess? the .svelte file or the test definition? ultimately the best dx would be to have it encapsulated in testing-library/svelte. requiring a custom bundler plugin or svelte preprocessor doesn't sound great. esp the component code under test must not be modified in any way that could change its behavior from production code... sourcemaps and step debugging tests could also be affected, so ideally we don't need custom transforms by default. opt-in transforms that reduce boilerplate, maaaaybe

dummdidumm added a commit that referenced this issue Feb 16, 2024
Introduces a new `hydrate` method which does hydration. Refactors code so that hydration-related code is treeshaken out when not using that method.
closes #9533
part of #9827
Rich-Harris added a commit that referenced this issue Feb 17, 2024
* feat: add hydrate method, make hydration treeshakeable

Introduces a new `hydrate` method which does hydration. Refactors code so that hydration-related code is treeshaken out when not using that method.
closes #9533
part of #9827

* get docs building

* ugh

* one more

* Update packages/svelte/scripts/check-treeshakeability.js

Co-authored-by: Ben McCann <322311+benmccann@users.noreply.github.com>

* warn

* Update sites/svelte-5-preview/src/routes/docs/content/01-api/05-functions.md

---------

Co-authored-by: Rich Harris <richard.a.harris@gmail.com>
Co-authored-by: Ben McCann <322311+benmccann@users.noreply.github.com>
Co-authored-by: Rich Harris <rich.harris@vercel.com>
dummdidumm added a commit that referenced this issue Feb 18, 2024
Rich-Harris added a commit that referenced this issue Feb 18, 2024
…ce `unmount` (#10516)

* breaking: remove `createRoot`, adjust `mount`/`hydrate` APIs, introduce `unmount`

closes #9827

* Update packages/svelte/src/internal/client/runtime.js

Co-authored-by: Simon H <5968653+dummdidumm@users.noreply.github.com>

---------

Co-authored-by: Rich Harris <richard.a.harris@gmail.com>
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 a pull request may close this issue.

5 participants