Skip to content
This repository has been archived by the owner on Jan 4, 2018. It is now read-only.

match to skate latest api and fix preact unmounting cleanup #6

Merged
merged 3 commits into from Dec 1, 2017

Conversation

Hotell
Copy link
Contributor

@Hotell Hotell commented Nov 19, 2017

Closes #5

  • applied preact hack

src/index.js Outdated
this._preactDom = render(
renderCallback(),
renderRoot,
this._preactDom || renderRoot.children[0]
);
}
disconnectedCallback() {
super.disconnectedCallback && super.disconnectedCallback();
this._preactDom = render(<div/>,this._renderRoot,this._preactDom);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

original is correctly replaced with empty div, although Preact won't call componentWillUnmount on mounted wrapped component ...

Copy link
Contributor Author

@Hotell Hotell Nov 19, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also it's not a shadowDOM thing. Following wont trigger willUnmount as well ( pure Preact )

import { h, Component, render } from 'preact'
class App extends Component {
  render(){
    return <div>original</div>
  }
  componentDidMount(){
    console.log('hi')
  }
  componentWillUnmount(){
    console.log('bye!')
  }
}
const mountTo = document.getElementById("root")
let root;

  root = render(<App />, mountTo);
  console.log(root)
  setTimeout(()=>{
   // element is replaced bud willUnmount won't be called ... Preact BUG ?
    render(<div>replaced</div>, mountTo, root)
  },1400)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I don't have any experience here. Might be worth pinging @developit about this? It looks like unmounting at a node is in compat, but I don't want to place a dependency on that preactjs/preact#53.

@Hotell
Copy link
Contributor Author

Hotell commented Nov 30, 2017

Re-review pls @treshugart

@Hotell Hotell changed the title WIP: Match to skate api Match to skate api Nov 30, 2017
@Hotell Hotell changed the title Match to skate api match to skate latest api and fix preact unmounting cleanup Nov 30, 2017
@treshugart
Copy link
Member

Cool, thanks @Hotell!

@treshugart treshugart merged commit acff61e into master Dec 1, 2017
@treshugart treshugart deleted the match-to-skate-api branch December 1, 2017 02:15
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants