-
-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Refactoring generator code #204
Conversation
… out of generator
Current coverage is 92.42% (diff: 95.29%)@@ master #204 diff @@
==========================================
Files 61 71 +10
Lines 1698 1663 -35
Methods 0 0
Messages 0 0
Branches 0 0
==========================================
- Hits 1570 1537 -33
+ Misses 128 126 -2
Partials 0 0
|
Removing the WIP tag as I think this is ready to merge. Should be a bit easier now to keep the two (and eventually, possible, more) compilers in sync as and when new features get added and bugs get squashed. |
I think the use of events here feels really strange. Other than that, I think the code organization is much better this way. |
This is far from finished, but I thought it worth explaining where I'm going with this before picking it up again tomorrow.
As per #202, there's a lot we can do to reduce duplication between the SSR and DOM compilers. If we ever add other compilers (canvas? webgl? native?) or make compilers pluggable (first-class MobX/Redux support?), that work would also benefit from some better-factored code generation.
My thinking is that we have a generic base
Generator
class, which is created with a set of visitors specific to each compiler. Things that need to happen for all compilers (gathering imports, figuring out whether we have computed properties, contextualising expressions, dealing with sourcemaps, etc) are built-in to the generator. Stuff that is specific to a compiler likecreateMountStatement
gets removed from the generator, and is instead handled as events (i.e. a visitor can dogenerator.fire('createMountStatement, {...})
, and the compiler can have a corresponding event listener).