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

feat: scoped include #226

Open
wants to merge 11 commits into
base: next
Choose a base branch
from
Open

feat: scoped include #226

wants to merge 11 commits into from

Conversation

fengzilong
Copy link
Member

Closes #196

测试结果

tests

新的测试用例

scoped include 相关的用例待编写,稍后补上

实现思路

{#inc this.$body with { foo: bar }}

$watch this.$body 和 表达式{ foo: bar },在变动时,通过 extra 传入 $scope

问题

但是有一个问题,由于不知道用的时候是否使用了foo,导致一些场景下存在性能问题

比如:

{#list items as item}
  {#inc this.$body with { foo: bar, item: item, index: item_index }}
{/list}
<Scope>
  { $scope.item }
</Scope>

这种情况下,虽然未使用 foo,但我们无法得知,当 bar 变化时,导致 { foo: bar, item: item, index: item_index } 发生变化,代码会重新执行 ctx.$compile 编译 this.$body,将整个 this.$body 替换掉,但这更新是多余的,不知道是不是有更好的实现方式可以避开这个问题

@fengzilong fengzilong requested a review from leeluolee October 21, 2018 13:31
@fengzilong fengzilong closed this Oct 22, 2018
@XGHeaven
Copy link

这个很难做到正确的处理吧,regular 没有类似 vue 和 mobx 使用链路的追踪,很难得知这个值是否被用到吧。

@fengzilong fengzilong reopened this Oct 22, 2018
@fengzilong
Copy link
Member Author

可以做到,之前是我实现有问题

@fengzilong
Copy link
Member Author

实现思路(更新)

以下面的代码为例

{#inc this.$body with { foo: bar }}

通过 extra 传入 $scope(基本思路)

  • $watch this.$body

    在变化时 重新编译 this.$body

  • $watch { foo: bar }

    在变化时,由于绑定关系(watcher)已经建立,更新extra即可,使得脏值检查触发对应的watcher

@leeluolee
Copy link
Member

@fengzilong

<Scope>
  { $scope.item }
</Scope>

Scope指的是个特殊组件,还是代指所有使用了scope param 的组件?

@fengzilong
Copy link
Member Author

代指用了scoped include的组件

@leeluolee
Copy link
Member

所以嵌套的那种,是没有获取外层$scope的能力的吧?

@fengzilong
Copy link
Member Author

fengzilong commented Oct 26, 2018

如果指定不同的名字可以获取的,但是如果是默认的$scope,会被就近的那个组件覆盖

比如

  1. 可以获取到parentScope
<Parent $scope="parentScope">
  <Child>
    { parentScope.item }
    { $scope.item }
  </Child>
</Parent>
  1. $scope.item会使用就近的Child组件
<Parent>
  <Child>
    { $scope.item }
  </Child>
</Parent>

dist/regular.js Outdated
@@ -2885,9 +2881,45 @@ return /******/ (function(modules) { // webpackBootstrap

// {{~}}
op.inc = op.include = function(){
var content = this.expression();
var first = this.ll(1);
Copy link
Member

Choose a reason for hiding this comment

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

这个貌似有问题,是假设了 使用 this.$body 就是fragment类型 么? 理论上使用this.$body 和使用 其他 fragment类型的变量是一样的处理方式。 比如 <Scope title = {~ <div> {title} </div>} /> 配和 {#inc title}

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member Author

@fengzilong fengzilong Oct 26, 2018

Choose a reason for hiding this comment

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

是指这里的一堆if判断嘛,这个是为了限制复杂表达式的
比如{#inc x + y with { item: item }},复杂表达式x + y不允许和scoped include一起使用

Copy link
Member Author

@fengzilong fengzilong Oct 26, 2018

Choose a reason for hiding this comment

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

这段if/else对后面的scoped include实现没帮助的,只影响下面的this.error,去掉也没事,目前的实现已经支持 {#inc title} 这种类型的fragment了

Copy link
Member

@leeluolee leeluolee Oct 26, 2018

Choose a reason for hiding this comment

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

可以精简一次pr的内容,即使有坑,但之前也是没这个逻辑的,可以换个pr处理。

Copy link
Member Author

Choose a reason for hiding this comment

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

done

@fengzilong
Copy link
Member Author

合之前我再补充下测试用例吧

@fengzilong
Copy link
Member Author

done,测试用例已加

@MangoTsing
Copy link

scope的功能不打算加入master吗?

@fengzilong
Copy link
Member Author

scope的功能不打算加入master吗?

等我晚上回去先解决一下代码冲突

@fengzilong fengzilong requested a review from leeluolee July 31, 2020 05:11
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.

4 participants