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

feat: Forward all events (on:*) #8356

Closed
wants to merge 25 commits into from
Closed

Conversation

adiguba
Copy link
Contributor

@adiguba adiguba commented Mar 6, 2023

Hello,

I make a prototype that implement event forwarding via on:*, described in issue #2837
In order to work it require some rewrite of the way Svelte manager the event's listener.

The main changes are as follows :

  • on:event={handler} remove the listener when handler is null/undefined/invalid.
  • on:event|modifiers={handler} on component accepts the same modifiers as HTML node.
  • Forwarding with on:event add a listener on the node/component only when it's listened on the component.
  • Forwarding with on:event|alias allows us to forward an event under another name (alias).
  • Forwarding with on:* allow us to forward all events. In fact when any handler is registered to the component, it will be added to the sub node/component.
  • Forwarding with on:*|prefix-* or on:*|*-suffix will allows us to forward alls events using a prefix or suffix.
    This will allow us to distinctly forward all event from distinct node/component.
  • A new lifecycle function onEventListener that allows to manually manage an handler.

There are still some jobs required (including tests and docs), and I have problems on some type-checking error with Components.

I will add comments here with details about my implementation.

Before submitting the PR, please make sure you do the following

  • It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs
  • Prefix your PR title with feat:, fix:, chore:, or docs:.
  • This message body should clearly illustrate what problems it solves.
  • Ideally, include a test that fails without this PR but passes with it.

Tests

  • Run the tests with npm test and lint the project with npm run lint

@vercel
Copy link

vercel bot commented Mar 6, 2023

@adiguba is attempting to deploy a commit to the Svelte Team on Vercel.

A member of the Team first needs to authorize it.

@adiguba
Copy link
Contributor Author

adiguba commented Mar 6, 2023

on:event={handler} with null|undefined

Actually when the handler is null|undefined, it's still added to addEventListener() (who ignores it)

Buf if we use some modifiers (preventDefault, trusted, self, stopPropagation) that wrap the handler, it will be executed and they raise an exception.

REPL : https://svelte.dev/repl/2a222085ea2f4f25a2a04e1955608536?version=3.55.1

Actually, this code :

<a on:click|preventDefault={handler} ...>

Will actually generate a code like this :

	dispose = listen(a, "click", prevent_default(/*handler*/ ctx[0]));

In order to fix that, I make two change.

  • listen() will receive a fourth optionnal parameter, which contains an array of wrapper functions.
  • listen() will ignore all falsy handler, before applying any wrapper functions...

So this code will compile to :

	dispose = listen(a, "click", /*handler*/ ctx[0], [prevent_default]);

@adiguba
Copy link
Contributor Author

adiguba commented Mar 6, 2023

on:event={handler} with dynamic handler

A similar problem can occur when the handler is dynamic, given that it is wrapped in a function that will do the null-check.

So when handler is dynamic, this line :

<a on:click|preventDefault={handler} ...>

Will actually generate a code like this :

	dispose = listen(a, "click", prevent_default(function () {
		if (is_function(/*handler*/ ctx[0])) /*handler*/ ctx[0].apply(this, arguments);
	}));

With nothing on the update function of the component.
Event when the handler is invalid, it's still registered into the DOM node.

I changed that with a new lifecycle function listen_and_update with 2 parameters :

  • A function that return the current handler
  • A function that listen the handler.

When the component is mounted, the handler is registered like this :

	dispose = listen_and_update(
		() => /*handler*/ ctx[0],
		(h) => listen(a, "click", h, [prevent_default])
	);;

But the dispose has a populate method in order to change his handler.
So the #populate function of the component can change the handler, adjusting

	if (dirty & /*handler*/ 1) {
		dispose.p(/*handler*/ ctx[0]);
	}

In fact the listen_and_update() method is pretty simple :

export function listen_and_update(
	get_handler: ()=>EventListenerOrEventListenerObject | null | undefined | false,
	factory: (handler:EventListenerOrEventListenerObject | null | undefined | false) => Function) {

	let handler = get_handler();
	let dispose_handle: Function = factory(handler);
	const dispose = () => dispose_handle();
	// update :
	dispose.p = () => {
		const new_handler = get_handler();
		if (new_handler !== handler) {
			dispose_handle();
			handler = new_handler;
			dispose_handle = factory(handler);
		}
	};
	return dispose;
}

@adiguba
Copy link
Contributor Author

adiguba commented Mar 6, 2023

Event modifier on component's event

Actually the component's event only accept the once modifier.
Even preventDefault is not allowed despite the fact that it can be handled by dispatch().
And all the other modifiers cannot be passed, even for events that are forwarded to HTML nodes...

I've modified that in two way :

  • By adding a third parameter options to SvelteComponent.$on(), to match addEventListener()
  • By adding a lifecycle function listen_comp(), similar to listen() but for component.

The function $on() will ignore the options parameters and pass it to the addEventListener on the forwarded node.
Except for 'once' which needs to be addressed specifically.

@adiguba
Copy link
Contributor Author

adiguba commented Mar 6, 2023

Forwarding (on:event)

Actually when we forward an event, it adds a listener on the node which will take care of bringing it up via the component's dispatcher.

So in fact the following code :

<button on:click> click </button>

is compiled to this (like any other event handler) :

	dispose = listen(button, "click", /*click_handler*/ ctx[0]);

Except that the handler is generated by the Svelte compiler :

	function click_handler(event) {
		bubble.call(this, $$self, event);
	}

Where bubble is a function that dispatches the event to the handlers added to the component...

The major problem with this solution is that it adds a listener anyway, even if there is no handler on the component.
It is useless, but above all it prevents us from forwarding all the events of a node (we cannot add ALL the events !)

I've completly rewrote that and now bubble is used in place of listen to register the event-forward, without adding any listener on the node as long as the event is not tracked in the parent component :

	dispose = bubble($$self, listen, button, "click");

This method is mainly used to store info in the internal object $$, and the real listeners will be added/removed in partnership with the component's .$on() function.

It take 4 required arguments :

  • The current component.
  • The listen function to use for registering an handler.
  • The target of the handler.
  • The name of the event.

Finally, .$on() and bubble() will work together to add the required handlers at the right time.

@adiguba
Copy link
Contributor Author

adiguba commented Mar 6, 2023

Forwarding all events (on:*)

The same principle will be applied to on:*, which will be compiled to :

	dispose = bubble($$self, listen, node, "*");

But "*" will have a special meaning for bubble() and $on().
It will be associated with all the events added to the component.

So any event handler added to the component will be registered to the node.

@adiguba
Copy link
Contributor Author

adiguba commented Mar 6, 2023

Alias

In fact this new bubble() method accepts a fifth parameter, allowing to use an alias for the forward.

So something like on:click|clicked will forward the event 'click' when the event 'clicked' is registered on the component, and will be compiled to :

	dispose = bubble($$self, listen, button, "click", "clicked");

This require a special case for on:* where the alias name must start or end with '*' (ex on:*|prefix-* or on:*|*-suffix).

This may cause a breaking change as actually event's modifier can be use when forwarding (but I dont see real use-case for that, and I think it should be forbidden).

@adiguba
Copy link
Contributor Author

adiguba commented Mar 6, 2023

onEventListener()

All of this is managed via a new lifecycle method onEventListener(), with the following signature :

function onEventListener(
	type: string,
	fn: (
		callback: EventListener,
		options: boolean | AddEventListenerOptions | EventListenerOptions | undefined,
		type: string
	) => Function | undefined
) => void
  • type is the event name we want to process (or "*" for all event).
  • fn is a factory function responsible for registering the listener.

When a new valid handler is added to the component (via .$on() or a on:event={handler}) the fn function will be called in order to register the event or not :

  • if it handles the event, it must return an function to remove the listener.
  • otherwise it must return undefined

This is the heart of how on:* works, but could also be used to handle external events (like message from WebSocket...).
So I think it should be a public function available for final user.

@adiguba
Copy link
Contributor Author

adiguba commented Mar 6, 2023

A few remarks...

Non-breaking change on internal (non-public) function.

  • listen() and .on() have be modified/enhanced, but their behavior should be similar when used with their old signature.

  • The new mecanism between .$on() and bubble() require to re-create the listeners when the code is hot-reloaded on Dev mode.
    I actually use an internal API from svelte-hmr in order to do that - see Make $$.on_hmr public (or a way to detect HMR update) svelte-hmr#66

Breaking change

  • bubble() has a total different usage now.
    This method should not be called by user (but maybe i should use another name).

  • The event on component use listen_comp() on the #mount() method instead of .on() just after the component creation.
    It is possible that this has an impact on certain events. TO BE VERIFIED !

  • Actually some syntax like on:click|preventDefault or on:click|preventDefault={null} will block the event even if there is no handler. Same thing with on:click|preventDefault={handler} with a nullable dynamic handler.
    This is not the case anymore here...

  • Event modifier on event forwarding (ex: on:click|preventDefault) will raise a warning/error, since modifier should be used for the alias name in this context.

TODO

  • There are still some type-checking error with Component, as event forward via on:* or on:click|alias are badly declared on the Component type.
    I don't know how to fix that.

  • VS Code still produce a syntax error on on:* ...

  • Tests, docs, ...

Thanks for reading, and sorry for my poor english...

@adiguba
Copy link
Contributor Author

adiguba commented Aug 13, 2023

Hello,

There a lot of conflict with Svelte 4, so I just created another branch here : https://github.com/adiguba/svelte/tree/on-any-svelte4

@adiguba
Copy link
Contributor Author

adiguba commented Aug 18, 2023

Hello,

I just created a little stackblitz.com with an inegrated svelte library that use on:*

Can be tested here :

https://stackblitz.com/edit/on-any-4-svelte?file=src%2FApp.svelte

@adiguba
Copy link
Contributor Author

adiguba commented Dec 3, 2023

I think that this PR is obsolete with Svelte 5.

@adiguba adiguba closed this Dec 3, 2023
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 this pull request may close these issues.

2 participants