fixes #1188, fixes #1201

[ekhaled]

My first foray into svelte internals, so please forgive any stupidities.

I set out to fix #1188 , but managed to fix #1201 for free.

I have updated teh snapshots. However, I couldn't find any tests for transitions.

Let me know if I need to change anything

[codecov-io]

Codecov Report

Merging #1210 into master will decrease coverage by <.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #1210      +/-   ##
==========================================
- Coverage   91.64%   91.63%   -0.01%     
==========================================
  Files         127      127              
  Lines        4558     4557       -1     
  Branches     1503     1502       -1     
==========================================
- Hits         4177     4176       -1     
  Misses        159      159              
  Partials      222      222

Impacted Files	Coverage Δ
src/generators/dom/index.ts	96.21% <ø> (-0.03%)	⬇️

---

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 18d3313...f9d606a. Read the comment docs.

[ekhaled]

Found the transition tests 🤦‍♂️

[ekhaled]

Can't figure out why Travis complains. It should be able to find ./Child.html

[Rich-Harris]

What do you think about putting the || null inside the _mount method, rather than in calls to _mount? That way it only appears once per codebase (I'm not totally sure it's necessary at all, now that I think of it — though it might be in some browsers...)

Slightly orthogonal to the task at hand, but seems like as good a time as any to make that change now that we're introducing more calls to _mount.

[ekhaled]

wouldn't sending options.anchor without the || throw an undefined error if options.anchor is missing?

[Rich-Harris]

I meant that src/shared/index.ts would have this:

export function _mount(target, anchor) {
	this._fragment.m(target, anchor || null);
}

...and then the generated code could do this._mount(options.target, options.anchor) instead of needing the || null there

[ekhaled]

Done as suggested.

Sorry about the messy commit structure.. maybe squash and merge when you do.

While we are here,
I think we need a ternary like this in _unmount too. Because nothing in the codebase ever calls _fragment.o().
Components never transition out

[Rich-Harris]

Nice, thanks. Not sure what's going on with that test failure — could just be an innocent timeout, so have restarted.

Components never transition out

Yeah, that's kind of a tricky problem — one that we can kick down the road a bit I think

[ekhaled]

please wait before you merge
There is a case sensitivity problem still. let me fix that

[ekhaled]

ok.. looks ready to merge now 😥

Yeah, that's kind of a tricky problem — one that we can kick down the road a bit I think

wouldn't something like this in _unmount fix the problem:

if(this._fragment.o){
  this._fragment.o(() => this._fragment.u())
}else{
  this._fragment.u()
}

[Rich-Harris]

Thanks so much! Released 1.56.3.

Yeah, it might actually be as simple as that. I remember thinking there was a tricky coordination problem because the transitions are happening across 'levels' (i.e. there might be some fun stuff to take into account if you need to re-render the component while it's outroing) but I might be wrong about that. Opened #1211 anyway