Refactor

[Rich-Harris]

Just a few high-level thoughts for now, since I'm about to leave my desk but want to get this out in the open. I have a completely unscientific feeling that there's a better way to handle a lot of the code generation stuff, that will be more readable, more maintainable and perhaps even more extensible, eventually.

I'd like to create a new Fragment class (or maybe Block) that handles some of the work Generator currently does. At the moment, there's a lot of weird indirection, e.g. generator.addElement basically just hands over to the current fragment's init builder, which is a little confusing.

I also think that node visitors should be responsible for visiting their own children, since that makes it easier to control the order in which stuff happens, and makes it possible to pass parameters down to child nodes without awkwardly sticking properties on objects all over the place.

One of the immediate goals following this work is to hoist event handlers (where applicable), so this PR will need to bear that in mind (or include that work).

[PaulBGD]

Should any big changes wait for this?

[Rich-Harris]

@PaulBGD I think so, yes — sorry! Actively working on it (in the sense of writing lots of code and then immediately deleting it, these sorts of refactors are always a bit painful. Better to suffer that pain now though), hoping to have it ready soon... did you have any particular changes in mind?

[PaulBGD]

I'm just checking before I get invested into working on some more features, take your time though.

[Narretz]

Will this mean API changes? I was just about to try Svelte, but I haven't seen anywhere anything about API stability. I assume it follows semver?

[Rich-Harris]

No API changes, doubly so for the components it generates. This is just about moving some of the internal code generation logic around so that it's a bit better structured — at the moment some of the work happens in weird places, and I have a hunch that it would slow our ability to make improvements in the future if we don't wrestle with it now.

[Swatinem]

I also think that node visitors should be responsible for visiting their own children, since that makes it easier to control the order in which stuff happens, and makes it possible to pass parameters down to child nodes without awkwardly sticking properties on objects all over the place.

👍

Will look at this soon-ish, but the description so far sounds good.

[Rich-Harris]

Alright, there's probably some more stuff that can happen here but I think it's ready for merge — any extra improvements can be made on top of this, I think the 'structure' is basically where it needs to be.

Essentially, this clarifies a few internal concepts and moves some logic to more, well, logical places. Previously, as we walked the template AST, we tracked the current 'fragment' (which could be the template as a whole, an if/each block, or an element) as generator.current. Some of those fragments got turned into functions like render_main_fragment and render_if_block, but elements behaved differently. Meanwhile, fragments did a lot of extra work that didn't really belong, like keeping track of the current element namespace.

Instead, there's now a clean separation between blocks — things that result in render_main_fragmentetc being created — and state, like the current element namespace and the parentNode to which new elements should be appended.

Since node visitors are now responsible for visiting their own children, there's no need for hacks like fragment.children = [], and visitors are just functions (rather than {enter, leave} objects).

I renamed a few things for consistency, e.g. render_main_fragment is now create_main_fragment, and so on.

[Swatinem]

Looked at it quite superficially, but the changes look really good!

[Rich-Harris]

#456 is ready to go and depends on this, so I'll merge it in