-
-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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: functional template generation #15538
base: main
Are you sure you want to change the base?
Conversation
🦋 Changeset detectedLatest commit: a48df4a The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Should we create a helper function that lets us emit more compact code? The hello world currently looks like this: var root = $.template_fn(() => {
var h1 = document.createElement('h1');
var text = document.createTextNode('hello world');
h1.insertBefore(text, undefined)
var fragment = document.createDocumentFragment();
fragment.append(h1)
return fragment;
}); If function template_fn(flags, ...items) {
var fragment = document.createDocumentFragment();
// TODO get from flags
var ns;
for (var item of items) {
fragment.append(create_item(item, ns));
}
return fragment;
}
function create_item(item, ns) {
if (item === undefined) {
return document.createComment('');
}
if (typeof item === 'string') {
return document.createTextNode(item);
}
var name = item[0];
if (!ns) {
if (name === 'svg') return create_item(item, 'http://www.w3.org/2000/svg');
if (name === 'math') return create_item(item, 'http://www.w3.org/1998/Math/MathML');
}
var element = ns ? document.createElementNS(ns, name) : document.createElement(name);
for (var i = 1; i < item.length - 1; i += 2) {
element.setAttribute(item[i], item[i + 1]);
}
const children = item[i];
if (children) {
for (var child of children) {
element.append(create_item(child));
}
}
return element;
} ...then it could look like this instead: var root = $.template_fn(0, ['h1', ['hello world']]); On larger apps this would become significant. I might be glossing over some details of e.g. attributes vs props but you get the idea |
The point is that we should then substitute We could probably accept that as an input of the |
This would also move more computation at the runtime as opposed as the build time (probably not really noticeable but something to keep in mind nonetheless) |
This swap our
template.push
calls from bare strings to a series of instructions that looks like thisYou can use this playground link if you want to play around with the output.
for the moment i've basically written some code that generate the string for the template from this series of instructions but i'll work on a separate transformation to generate a series of document.createElement (and in the future renderer.createElement) from this series of instructions.
There's one thing that i don't properly like and it's the fact that there are those
push_element
andpop_element
instructions that don't translate into anything and just serve the purpose of delimit the children...but this allow us to not pass around variables and just refer to "the last element"Update
I've also included the transformer and the option to convert with string template or with functional template and updated the test suite to run with both modes.
The new option will look like this
and the code produced looks like this.
If you want to play around with it here's a stackblitz with everything setup.
Also since this is quite a big PR if you want i can split it in two (even tho it's already separated by commits well enough)
Before submitting the PR, please make sure you do the following
feat:
,fix:
,chore:
, ordocs:
.packages/svelte/src
, add a changeset (npx changeset
).Tests and linting
pnpm test
and lint the project withpnpm lint