-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
fix(runtime): 组件属性设置false在小程序环境不生效 #5990
Conversation
@@ -105,13 +105,19 @@ export class TaroElement extends TaroNode { | |||
} | |||
|
|||
public removeAttribute (qualifiedName: string) { | |||
const isVue = process.env.FRAMEWORK === 'vue' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这个变量是不需要的,直接写 process.env.FRAMEWORK === 'vue'
,如果为 false,webpack 会在生产模式编译的时候直接删除对应的条件语句/表达式,写了变量反而不一定做得到。(webpack 的求值和作用域处理还不够智能)
@@ -105,13 +105,19 @@ export class TaroElement extends TaroNode { | |||
} | |||
|
|||
public removeAttribute (qualifiedName: string) { | |||
const isVue = process.env.FRAMEWORK === 'vue' | |||
const internalComponent = internalComponents[toCamelCase(capitalize(this.tagName.toLowerCase()))] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这里应该移动到 // avoid attribute being removed because set false value in vue
的分支里去,我们不希望在非 vue 的情况也执行这个语句和其中包含的函数。
另外先 toCamelCase 再 capitalize 比较稳健,因为 capitalize 已经没有优化的空间了,toCamelCase 以后可能还会改。
另外你可以用 in
操作符把这个变量变成一个布尔值
@@ -105,13 +105,19 @@ export class TaroElement extends TaroNode { | |||
} | |||
|
|||
public removeAttribute (qualifiedName: string) { | |||
const isVue = process.env.FRAMEWORK === 'vue' | |||
const internalComponent = internalComponents[toCamelCase(capitalize(this.tagName.toLowerCase()))] | |||
const isBooleanType = val => val === 'true' || val === 'false' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这个函数不是闭包函数,他应该提出来到 shared 包中,一方面是为了性能,另外一方面在别的地方也能用到。他的名字准确来说应该是 isBooleanStringLiteral
.
感谢,你的另一个 PR #5978 也 review 了。 |
packages/shared/src/components.ts
Outdated
@@ -642,7 +642,11 @@ export function createMiniComponents (components: Components, buildType: string) | |||
} else if (propValue === '') { | |||
propValue = `i.${toCamelCase(prop)}` | |||
} else { | |||
propValue = `i.${toCamelCase(prop)} || ${propValue || singleQuote('')}` | |||
if (propValue === 'true' || propValue === 'false') { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这里接着上方的 propValue === ''
的条件判断就行了,不用再开一组判断
优化属性值为false时的取值表达式,适配当属性值为falsy时vue中remove attribute逻辑
已修改 |
仔细想了解决问题 2 的办法也不是很 ideal,因为要多引入一个很大的对象,但 vue 的做法其实也没错,因为所有 web 组件所有 falsy value 的属性都相当于不添加属性,但对小程序而言就不是这样。 以后考虑在模板生成的时候针对 vue 处理,现在先这样吧。 |
是的,还有当值为truthy value的时候,web会将属性值设置为key,这里和小程序里的处理逻辑也不同,不过这个目前不会造成判断异常; 另外这个大对象引入是指 |
改为这样应该比较合理,不用引入 当 Vue 框架调用 副作用是设置 |
这个 PR 做了什么? (简要描述所做更改)
修复#5988 小程序组件设置false不生效问题。
排查发现两个问题:
第一个问题
当属性值类型是布尔类型,生成的expression binding为造成异常,react和vue都不会生效;
如以下:
当开发者需要设置i.controls为false,不会生效;
第二个问题
当使用
vue
框架时,如果设置属性值为false value,则属性会被remove,造成上面例子i.controls
拿到的是undefined
。vue
处理逻辑:https://github.com/vuejs/vue/blob/0fb03b7831693b4abc90dd0bfe971c36c02d82a6/src/platforms/web/runtime/modules/attrs.js#L68且和
@tarojs/react
的处理逻辑不一样:https://github.com/NervJS/taro/blob/next/packages/taro-react/src/props.ts#L129
初步解决思路有两个:
undefined
时,判断开发者是设置false还是没有设置改属性的场景,如果是主动设置false,在生成表达式时做处理第一个思路想了下,因为
undefined
处理是在vue框架层,暂时没有想到办法从taro侧判断是开发者没有设置该属性还是主动设置为false;因此目前采用第2个思路解决;这个 PR 是什么类型? (至少选择一个)
这个 PR 满足以下需求:
这个 PR 涉及以下平台:
其它需要 Reviewer 或社区知晓的内容: