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

踩坑记 #20

Open
HecateDK opened this issue Mar 22, 2018 · 1 comment
Open

踩坑记 #20

HecateDK opened this issue Mar 22, 2018 · 1 comment

Comments

@HecateDK
Copy link
Owner

错误修复popover组件 bug总结

背景

react-gm的popover组件,当浏览器滚动时,一般popover跟随元素滚动;但如果元素是position: fixed,则popover固定住。

此时,就需要框架层Framework去监听浏览器的滚动状态,这里我们使用了EventEmitter——事件触发与事件监听器功能。

代码如下:

    // framework.js
    componentDidMount() {
        window.addEventListener('scroll', _.throttle(this.doScroll, 200));  
    }
    doScroll = () => {
        Emitter.emit(Emitter.TYPE.BROWSER_SCROLL);
    };
    
    // popover.js
    componentDidMount() {
        Emitter.on(Emitter.TYPE.BROWSER_SCROLL, _.debounce(this.handleBrowserScroll, 200));
    }
    componentWillUnmount() {
        Emitter.off(Emitter.TYPE.BROWSER_SCROLL, this.handleBrowserScroll);
    }
    handleBrowserScroll() {
        // 对popover组件进行定位计算
    }

若页面使用了popover组件,只要一滚动浏览器,控制台不断console.warning:
image

错误修复bug

当初看这个warning的时候,虽然google搜了这方面的bug,但认定是:

popover组件已经注销了,但framework一直在监听浏览器滚动,而浏览器滚动则一直在触发handleBrowserScroll方法,进而对不存在的popover组件进行setState操作。

后续也往这方面思考解决方法:

popover组件实例化或者注销时,使用EventEmitter限定framework监听浏览器滚动的时限
    // popover.js
    componentDidMount() {
        // 增加一个EventEmitter
        Emitter.emit(Emitter.TYPE.POPOVER_EXIST);
    }
    
    // framework.js
    componentDidMount() {
        // 当popover组件存在时,才监听浏览器的滚动
        Emitter.on(Emitter.TYPE.POPOVER_EXIST, () => {
            window.addEventListener('scroll', _.throttle(this.doScroll, 200)); 
        });
    }

当然,这个并不能奏效,不管是判断popover组件存在时才去监听浏览器的滚动,还是popover组件注销时,framework里window.removeEventListener

隐藏规避掉这个bug
    handleBrowserScroll() {
        if(this.refPopup) {    // 如果popover组件渲染出来了
           this.setActive(this.state.active);
        }
    }

由于我认为这个warning是对“不存在的popover组件进行setState操作”造成的,所以这样确实能够解决bug。

但。。。被骂了一顿。哈哈哈

错误原因分析

  • 因为Emitter.on(Emitter.TYPE.BROWSER_SCROLL,_.debounce(this.handleBrowserScroll, 200));是copy大佬另一个类似逻辑的代码,所以潜意识里面认为这句代码肯定不会出错。
  • 没有理解这个warning的根本原因,只是看到了表面,所以很简单的加了一个if(refPopup)来隐藏掉这个bug
  • 没有事件广播与事件监听的思想:
    本例子中,framework是事件广播方,一直监听着浏览器的滚动,广播给各个事件监听方(这就是popover组件)。framework会一直广播事件,不能其中一个监听方是否接收广播,不能把广播通道给封住。
  • 没有理解componentWillUnmount里,Emitter.off(Emitter.TYPE.BROWSER_SCROLL, this.handleBrowserScroll);的含义
    这是popover组件注销时,把handleBrowserScroll方法给注销掉。所以如果程序执行到了这里,并且Emitter.off没有问题的话,可以考虑的就是handleBrowserScroll有没有被成功注销。
  • 当遇到一个理解不了的bug时,不能静下心来,弄清楚逻辑,一行行代码去debug

解决方案

当错误原因定位到Emitter.off(Emitter.TYPE.BROWSER_SCROLL, this.handleBrowserScroll);上时,就去思考为什么this.handleBrowserScroll没有被注销掉。

Emitter.off注销的事件要求和Emitter.on监听的事件一样,那么就去对比一下两者代码差异:

    // popover.js
    componentDidMount() {
        Emitter.on(Emitter.TYPE.BROWSER_SCROLL, _.debounce(this.handleBrowserScroll, 200));
    }
    componentWillUnmount() {
        Emitter.off(Emitter.TYPE.BROWSER_SCROLL, this.handleBrowserScroll);
    }

对的,Emitter.on的时候,用的是lodashdebounce方法:

image

当然,这个bug是我大佬找出来的,并且他说_.debounce(abc, 200) !== abc时,我还理解不了。。。。

总结

  • 别只是照搬他人的代码,把他人的bug都给搬过来
  • 不能潜意识认为他人所写的代码是没有bug的
  • 不能规避问题,把bug隐藏掉,会埋下很严重的坑的
  • 好好debug
@liyatang
Copy link

点赞。然而不能通俗易懂的表达出来...

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

No branches or pull requests

2 participants