Skip to content

Add Event Delegation to Svelte Implementation #561

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.

Sign up for GitHub

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

Closed
wants to merge 1 commit into from
Closed

Add Event Delegation to Svelte Implementation #561

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 14 additions & 8 deletions frameworks/keyed/svelte/src/Main.svelte → frameworks/keyed/svelte/src/Main.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,14 +30,12 @@ <h1>Svelte (keyed)</h1>
</div>
</div>
<table class="table table-hover table-striped test-data">
<tbody>
<tbody on:click={click}>
{#each data as row, num (row.id)}
<tr class={selected === row.id ? 'danger' : ''}>
<td class="col-md-1">{row.id}</td>
<td class="col-md-4">
<a on:click={() => select(row.id)}>{row.label}</a>
</td>
<td class="col-md-1"><a on:click={() => remove(row.id)}><span class="glyphicon glyphicon-remove" aria-hidden="true"></span></a></td>
<td class="col-md-4"><a>{row.label}</a></td>
<td class="col-md-1"><a><span class="glyphicon glyphicon-remove" aria-hidden="true"></span></a></td>
<td class="col-md-6"></td>
</tr>
{/each}
Expand All @@ -60,8 +58,8 @@ <h1>Svelte (keyed)</h1>
data[i].label += ' !!!';
}
},
remove = (num) => {
const idx = data.findIndex(d => d.id === num);
remove = id => {
const idx = data.findIndex(d => d.id === id);
data = [...data.slice(0, idx), ...data.slice(idx + 1)];
},
run = () => {
Expand All @@ -72,11 +70,19 @@ <h1>Svelte (keyed)</h1>
data = buildData(10000);
selected = undefined;
},
select = (id) => selected = id,
select = id => selected = id,
swapRows = () => {
if (data.length > 998) {
data = [data[0], data[998], ...data.slice(2, 998), data[1], data[999]];
}
},
click = ({ target }) => {
const id = Number.parseInt(target.closest("tr").firstChild.firstChild.nodeValue);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ryansolid

so, we're still doing this, huh?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair is fair? I added implicit Event Delegation to my libraries to align with what the VDOM implementations. And I like the story it tells for code aesthetics ("Write less code"), but it is hardly a fair comparison when implementations aren't allowed to use the exact same mechanisms.

I realize the same could be argued about any trick VanillaJS does, but it just means the floor is pretty low. It's the same old discussion again. Which for me comes down to how does the implementation want to present itself. As long as the minimum requirements are met (Ie.. completely data driven, ie renderable from non-DOM state at any given point) it is up to them how nasty and complicated they want to make their implementation.

So yes I was hesitant to do this initially. Especially without @Rich-Harris approval since I get the impression Svelte is a library that cares more about aesthetics than performance. But absent of that I'd say it's probably fair to align this with other high performing non-VDOM implementations like Surplus, LitHTML, LighterHTML, and so on.

As always ready to open this debate again. But I think it is very difficult for any implementation/implementor that cares about performance and doesn't include implicit event delegation in their libraries for various reasons, to even understand why this is an issue in itself. I implemented implicit event delegation and I still don't.

if (target.matches(".glyphicon-remove")) {
remove(id);
} else {
select(id);
}
};

function _random (max) { return Math.round(Math.random() * 1000) % max; };
Expand Down
2 changes: 1 addition & 1 deletion frameworks/keyed/svelte/src/main.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import Main from './Main.svelte';
import Main from './Main.html';

export default new Main({
target: document.querySelector( '#main' )
Expand Down
22 changes: 14 additions & 8 deletions frameworks/non-keyed/svelte/src/Main.svelte → frameworks/non-keyed/svelte/src/Main.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,14 +30,12 @@ <h1>Svelte (non-keyed)</h1>
</div>
</div>
<table class="table table-hover table-striped test-data">
<tbody>
<tbody on:click={click}>
{#each data as row, num}
<tr class={selected === row.id ? 'danger' : ''}>
<td class="col-md-1">{row.id}</td>
<td class="col-md-4">
<a on:click={() => select(row.id)}>{row.label}</a>
</td>
<td class="col-md-1"><a on:click={() => remove(row.id)}><span class="glyphicon glyphicon-remove" aria-hidden="true"></span></a></td>
<td class="col-md-4"><a>{row.label}</a></td>
<td class="col-md-1"><a><span class="glyphicon glyphicon-remove" aria-hidden="true"></span></a></td>
<td class="col-md-6"></td>
</tr>
{/each}
Expand All @@ -60,8 +58,8 @@ <h1>Svelte (non-keyed)</h1>
data[i].label += ' !!!';
}
},
remove = (num) => {
const idx = data.findIndex(d => d.id === num);
remove = id => {
const idx = data.findIndex(d => d.id === id);
data = [...data.slice(0, idx), ...data.slice(idx + 1)];
},
run = () => {
Expand All @@ -72,11 +70,19 @@ <h1>Svelte (non-keyed)</h1>
data = buildData(10000);
selected = undefined;
},
select = (id) => selected = id,
select = id => selected = id,
swapRows = () => {
if (data.length > 998) {
data = [data[0], data[998], ...data.slice(2, 998), data[1], data[999]];
}
},
click = ({ target }) => {
const id = Number.parseInt(target.closest("tr").firstChild.firstChild.nodeValue);
if (target.matches(".glyphicon-remove")) {
remove(id);
} else {
select(id);
}
};

function _random (max) { return Math.round(Math.random() * 1000) % max; };
Expand Down
2 changes: 1 addition & 1 deletion frameworks/non-keyed/svelte/src/main.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import Main from './Main.svelte';
import Main from './Main.html';

export default new Main({
target: document.querySelector( '#main' )
Expand Down