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

refactor #1367

Merged
merged 86 commits into from Apr 29, 2018
Merged

refactor #1367

merged 86 commits into from Apr 29, 2018

Conversation

Rich-Harris
Copy link
Member

See #1316.

Lots more work to do here, but I'm optimistic about the direction it's going in. Essentially the goals are:

  • speed up compilation, by
    • reducing the number of times the compiler has to walk the tree
    • not changing prototypes of objects after they've been created
    • being more disciplined about maintaining object shapes
    • not needing to clone the AST
  • DRY out certain bits of code
  • make the codebase slightly easier to dive into, particularly by getting rid of all the bewildering contextualise stuff
  • stretch goal: unify DOM and SSR compilers

I found myself leaning towards a couple of naming changes — internally, generator is referred to more simply as compiler, and in the generated code, state becomes ctx, since it's more appropriate to speak of a 'context' once you're inside each/await blocks.

@codecov-io
Copy link

codecov-io commented Apr 28, 2018

Codecov Report

Merging #1367 into master will decrease coverage by 0.38%.
The diff coverage is 92.33%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1367      +/-   ##
==========================================
- Coverage   91.49%   91.11%   -0.39%     
==========================================
  Files         126      120       -6     
  Lines        4469     4421      -48     
  Branches     1384     1360      -24     
==========================================
- Hits         4089     4028      -61     
- Misses        154      156       +2     
- Partials      226      237      +11
Impacted Files Coverage Δ
src/utils/flattenReference.ts 71.42% <0%> (-5.5%) ⬇️
src/validate/html/index.ts 97.95% <100%> (+0.08%) ⬆️
src/compile/nodes/Head.ts 100% <100%> (ø)
src/compile/dom/Block.ts 95.83% <100%> (ø)
src/compile/nodes/PendingBlock.ts 100% <100%> (ø)
src/compile/nodes/Binding.ts 96.03% <100%> (ø)
src/compile/nodes/EventHandler.ts 100% <100%> (ø)
src/compile/nodes/Comment.ts 100% <100%> (ø)
src/compile/nodes/ElseBlock.ts 100% <100%> (ø)
src/compile/nodes/shared/TemplateScope.ts 100% <100%> (ø)
... and 59 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a7c1c38...01c3558. Read the comment docs.

@Rich-Harris
Copy link
Member Author

At long last, the diff is now more red than green. Will probably find a few more things to tidy up but I reckon this is basically ready — going to remove the [WIP] label.

Most apps should get slightly smaller as a result of this PR, due to the way context is kept up to date.

@Rich-Harris Rich-Harris changed the title [WIP] refactor refactor Apr 28, 2018
@Rich-Harris
Copy link
Member Author

@Conduitry ah, good catch, thanks

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.

None yet

3 participants