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

Release/1.1.0 #27

Merged
merged 13 commits into from
Nov 13, 2019
Merged

Release/1.1.0 #27

merged 13 commits into from
Nov 13, 2019

Conversation

daysai
Copy link
Contributor

@daysai daysai commented Nov 6, 2019

变更点

  • fix: 从安全性考虑,删除 localUrl api
  • feat: 为 AppRoute 添加 render/component props,支持替代 react-router 的基本能力 AppRouter/AppRoute 兼容 react-router 基本能力 #25
  • chore: 调整 testing-library 包 chore: migrate testing-library #22
  • fix: 调整 NotFound 渲染为 AppRouter 层渲染,逻辑上更合理,后续逐步调整 Loading/Error
  • fix: 完善测试用例,目前测试用例覆盖 95%+,遗留 shadowRoot 相关、自定义事件的 polyfill 相关

计划发布时间

  • 2019.11.08

@@ -60,8 +60,6 @@ function getAppConfig(appRouteProps: AppRouteProps): AppConfig {
'useShadow',
'ErrorComponent',
'LoadingComponent',
'NotFoundComponent',
'useShadow',
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
Contributor Author

Choose a reason for hiding this comment

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

useShadow 重复了,NotFoundComponent 的渲染逻辑放到 AppRouter 层了,原来都交由 AppRoute 自行处理,所以需要透传 NotFoundComponent 属性。 改之后渲染层级上更合理,后续 LoadingComponent、NotFoundComponent 逐步迁移过去(这块迁移成本稍微高一点,预计1.2.0版本完成)

src/AppRoute.tsx Outdated
useShadow?: boolean;
ErrorComponent?: any;
LoadingComponent?: any;
NotFoundComponent?: any;
component?: React.ReactElement;
render?: () => React.ReactElement;
Copy link
Member

Choose a reason for hiding this comment

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

为什么需要同时支持 component 和 render?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

为什么需要同时支持 component 和 render?

保持跟 react-router 统一,component 意义在于能自给自足的组件,render 支持透传自定义属性,比如 iframe 的 src 等。

@@ -39,7 +39,7 @@ function loadAsset(
element.addEventListener(
'error',
() => {
callback(isCss ? undefined : new Error(`JS asset loaded error: ${url}`));
callback(isCss ? undefined : `JS asset loaded error: ${url}`);
Copy link
Member

Choose a reason for hiding this comment

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

JS asset -> js assets

Copy link
Contributor Author

Choose a reason for hiding this comment

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

JS asset -> js assets

JS asset -> js asset 因为报错是针对某一条 js

const { path, basename } = element.props as AppRouteProps;
const { path, basename, render, component } = element.props as AppRouteProps;

const commonProps = { location: { pathname, query, hash }, match, history: appHistory };
Copy link
Member

Choose a reason for hiding this comment

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

明确下 render 里需要哪些参数?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

明确下 render 里需要哪些参数?

目前保持跟 react-router 常用属性一致,location、match、history 等常用属性/方法

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

Choose a reason for hiding this comment

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

评估是否需要透出 appHistory,个人建议先不要透出。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

评估是否需要透出 appHistory,个人建议先不要透出。

从以往 react-router 的开发经验上,location、match、history 是必备条件。component 和 render 功能的开发,主旨也是支持框架应用替代 react-router 的基本功能,因此这三个优先选择透出。

package.json Outdated
@@ -1,6 +1,6 @@
{
"name": "@ice/stark",
"version": "1.0.0",
"version": "1.0.1",
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
Contributor Author

Choose a reason for hiding this comment

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

改一下

done

src/AppRoute.tsx Outdated
useShadow?: boolean;
ErrorComponent?: any;
LoadingComponent?: any;
NotFoundComponent?: any;
component?: React.ReactElement;
render?: (props?: object) => React.ReactElement;
Copy link
Member

Choose a reason for hiding this comment

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

这个不应该是 object,把 commonProps 的类型声明提上来

Copy link
Contributor Author

Choose a reason for hiding this comment

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

这个不应该是 object,把 commonProps 的类型声明提上来

done

@@ -51,13 +52,25 @@ function getHashPath(hash: string = '/'): string {
return searchIndex === -1 ? hashPath : hashPath.substr(0, searchIndex);
}

/**
* Render Component
Copy link
Member

Choose a reason for hiding this comment

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

这种注释无意义,要么就 Render Component, support component={Home} and component={<Home />}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

这种注释无意义,要么就 Render Component, support component={Home} and component={<Home />}

改为 Render Component, compatible with Component and 从函数功能上进行注释

@imsobear
Copy link
Member

ci 这个 warning 是什么?

image

@daysai
Copy link
Contributor Author

daysai commented Nov 13, 2019

ci 这个 warning 是什么?

image

这就是 ErrorComponent、LoadingComponent 的渲染在 AppRoute 层,也是通过 ReactDOM.render() 方式渲染的。注销也是通过 unmountComponentAtNode 方式注销的。所以会有这些 warning。第一个可以通过删掉 cleanup-after-each 解决

@imsobear imsobear merged commit 619b175 into master Nov 13, 2019
@delete-merged-branch delete-merged-branch bot deleted the release/1.1.0 branch November 13, 2019 03:54
wisdomofgod pushed a commit that referenced this pull request Apr 16, 2020
* fix: rm localUrl api

* fix: rm localUrl

* chore: migrate testing-library

ref testing-library/dom-testing-library#260

* feat: add AppRoute render/component props

* chore: rm setupFilesAfterEnv

* fix: adjust for cr

* fix: adjust JS asset to js asset

* fix: rm useless cleanup-after-each

* feat: add AppRouteComponentProps ts

* chore: update version

* fix: interface match to Match
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.

3 participants