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

Dynamic route #150

Closed
wants to merge 7 commits into from
Closed

Dynamic route #150

wants to merge 7 commits into from

Conversation

WhatAKitty
Copy link
Contributor

No description provided.

@afc163
Copy link
Member

afc163 commented Nov 9, 2017

Great job!

  • We should consider adding loading status for this. @sorrycc

@afc163
Copy link
Member

afc163 commented Nov 9, 2017

@WhatAKitty Could you help us updating the relevent document meanwhile?

https://pro.ant.design/docs/layout-cn
https://pro.ant.design/docs/router-and-nav-cn

@WhatAKitty
Copy link
Contributor Author

@afc163 ok, I will.

@WhatAKitty
Copy link
Contributor Author

I have updated doc just now.

@@ -142,7 +142,7 @@ class BasicLayout extends React.PureComponent {
<Link to={itemPath} target={item.target}>
{icon}<span>{item.name}</span>
</Link>
)
)
Copy link
Member

Choose a reason for hiding this comment

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

?

@afc163 afc163 mentioned this pull request Nov 10, 2017
@WhatAKitty
Copy link
Contributor Author

@afc163 Updated.

@ityao
Copy link

ityao commented Nov 10, 2017

啥时候更新上去呢, 正要启动一个新项目呢

}, {
path: 'result',
component: Step3,
component: app => dynamic({
Copy link

@sbyps sbyps Nov 10, 2017

Choose a reason for hiding this comment

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

here should return <Component /> rather than a dynamic function ,so this error occurs:
Functions are not valid as a React child. This may happen if you return a Component instead of from render. Or maybe you meant to call this function rather than return it.

Copy link
Contributor Author

@WhatAKitty WhatAKitty Nov 10, 2017

Choose a reason for hiding this comment

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

I cannot reproduce this issue,Could you show your code ?

Copy link

Choose a reason for hiding this comment

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

就是copy你的代码的pull request啊

Copy link
Contributor Author

@WhatAKitty WhatAKitty Nov 10, 2017

Choose a reason for hiding this comment

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

https://github.com/WhatAKitty/ant-design-pro please try again. This repo is ok and works good.

Copy link
Contributor Author

@WhatAKitty WhatAKitty Nov 10, 2017

Choose a reason for hiding this comment

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

And please note: If you use webpack instead of dva rodhog, please install [email protected]. the [email protected] version fix import issue

Copy link

@sbyps sbyps Nov 10, 2017

Choose a reason for hiding this comment

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

找到原因了,子组件写错了,Form.create()(Login)写成了Form.create(Login),抱歉

@afc163
Copy link
Member

afc163 commented Nov 13, 2017

I will handle this after dvajs/dva#1332

@ityao
Copy link

ityao commented Nov 13, 2017

image
改成按需加载后好像css加载顺序变了, 弄得左上方logo旁边的文字和logo不在一个水平线上

@afc163
Copy link
Member

afc163 commented Nov 14, 2017

Merged in d93b0bf

@afc163 afc163 closed this Nov 14, 2017
@afc163
Copy link
Member

afc163 commented Nov 14, 2017

@ityao This problem is resolved in 63b7645

@afc163
Copy link
Member

afc163 commented Nov 14, 2017

@WhatAKitty 现在有个性能问题,左边菜单展开收起时,右侧内容会整体重新渲染(触发 componentDidMount)。

@WhatAKitty
Copy link
Contributor Author

@afc163 这个问题我没察觉到,我看下。不过是否可能因为动态加载未判断不需要刷新页面的情况?

@afc163
Copy link
Member

afc163 commented Nov 14, 2017

不过是否可能因为动态加载未判断不需要刷新页面的情况

啥?

@WhatAKitty
Copy link
Contributor Author

@afc163 就是是否可以通过生命周期的componentWillReceiveProps来判断是否需要刷新页面的情况

@afc163
Copy link
Member

afc163 commented Nov 14, 2017

不应该这么做,页面代码不应该关心这个。

component={item.component(app)}

可能是这里用了函数调用的缘故。

@WhatAKitty
Copy link
Contributor Author

WhatAKitty commented Nov 14, 2017

@afc163 想到一个办法,在routeConfig内部事先完成navdata的初始化,然后将其传入到各个layout里面就行了。

// 我把这个方法直接提取到了router.js里面,没必要放到util内。直接可以在最顶层做统一规划处理
function getRouteData(navData, path) {
  if (!navData.some(item => item.layout === path) ||
      !(navData.filter(item => item.layout === path)[0].children)) {
    return null;
  }
  const dataList = cloneDeep(navData.filter(item => item.layout === path)[0]);
  const nodeList = getPlainNode(dataList.children);
  return nodeList;
}

function RouterConfig({ history, app }) {
  const navData = getNavData(app);
  const BasicLayout = dynamic({
    app,
    models: () => [
      import('./models/user'),
    ],
    component: () => import('./layouts/BasicLayout'),
  });
  const UserLayout = dynamic({
    app,
    component: () => import('./layouts/UserLayout'),
  });

  const passProps = {
    app,
    navData,
    getRouteData: (path) => {
      return getRouteData(navData, path);
    }
  }

  return (
    <LocaleProvider locale={zhCN}>
      <Router history={history}>
        <Switch>
          <Route path="/user" render={props => <UserLayout {...props} {...passProps} />} />
          <Route path="/" render={props => <BasicLayout {...props} {...passProps} />} />
          <Redirect to="/" />
        </Switch>
      </Router>
    </LocaleProvider>
  );
}

common.nav.js

const data = app => [{
  component: dynamic({
    app,
    models: () => [
      import('../models/user'),
    ],
    component: () => import('../layouts/BasicLayout'),
  }),
  layout: 'BasicLayout',
  name: '首页', // for breadcrumb
  path: '/',
  children: [{
    name: 'Dashboard',
    icon: 'dashboard',
    path: 'dashboard',
    children: [{
      name: '分析页',
      path: 'analysis',
      component: dynamic({
        app,
        models: () => [
          import('../models/chart'),
        ],
        component: () => import('../routes/Dashboard/Analysis'),
      }),
    }, {
    // more....

}, {
  component: dynamic({
    app,
    component: () => import('../layouts/BlankLayout'),
  }),
  layout: 'BlankLayout',
  children: {
    name: '使用文档',
    path: 'http://pro.ant.design/docs/getting-started',
    target: '_blank',
    icon: 'book',
  },
}];

export function getNavData(app) {
  return data(app);
}

// 移除直接暴露data数据
// export data;

测试了一下,不会重复加载了。

WhatAKitty added a commit to WhatAKitty/ant-design-pro that referenced this pull request Nov 14, 2017
WhatAKitty added a commit to WhatAKitty/ant-design-pro that referenced this pull request Nov 14, 2017
afc163 pushed a commit that referenced this pull request Nov 15, 2017
WhatAKitty added a commit to WhatAKitty/ant-design-pro-site that referenced this pull request Nov 16, 2017
nikogu pushed a commit to ant-design/ant-design-pro-site that referenced this pull request Nov 20, 2017
* Update doc with dynamic router support

* Load demand

* Change doc for ant-design/ant-design-pro#150

* Documentation with dynamicWrapper
@sbyps
Copy link

sbyps commented Nov 29, 2017

@WhatAKitty 每次点击Link组件发生路由变化都会导致routes中的组件重新挂载,生命周期从didMount重新执行了数据获取等操作,这是react-router4引入的么,怎么避免重新挂载呢,现在需要使用tab进行页面的切换,同时保持各个tab页面中state不变化

@sbyps
Copy link

sbyps commented Nov 29, 2017

@afc163 我fetch了master的最新代码在本地调试,在https://preview.pro.ant.design/#/dashboard/analysis 也试了一下,还是会重新fetch数据啊

@WhatAKitty
Copy link
Contributor Author

@sbyps 并不会重新加载啊。preview上也没看到有重新fetch数据。组件本身,在动态引入后,不会重新加载第二次,而是会缓存这个组件。

@sbyps
Copy link

sbyps commented Nov 29, 2017

@WhatAKitty 现在正在本地调试呢,在左侧菜单切换时,每个页面的componentDidMount又执行了,有其他联系方式么

@WhatAKitty
Copy link
Contributor Author

WhatAKitty commented Nov 29, 2017

@sbyps 切换菜单的话,componentDidMount的确会执行,不过这个是正常现象吧。

@sbyps
Copy link

sbyps commented Nov 29, 2017

@WhatAKitty 按照我理解mount一次就好了啊,那不是从头又执行了一整遍生命周期。这样也多次请求了数据,触发了model的变化

@WhatAKitty
Copy link
Contributor Author

@sbyps 这个的确是个问题,性能上考虑,最好第一次加载完毕后,后几次仍旧使用之前的实例。数据的变更通过componentWillReceiveProps来进行setState

@WhatAKitty
Copy link
Contributor Author

WhatAKitty commented Nov 29, 2017

@afc163 @nikogu 这种加载形式需要涉及dynamic,是否考虑使用https://github.com/faceyspacey/react-universal-component 替换dva的动态加载?

@WhatAKitty
Copy link
Contributor Author

WhatAKitty commented Nov 29, 2017

@sbyps 目前来说,重新渲染一遍花费的代价并不大,引用一篇文章的描述:

One thing to note about this is its performance characteristics. This actually does less work at “render time,” since the components are pre-created. You’re essentially sacrificing memory for CPU at a very critical point in time. I haven’t measured how many cycles/milliseconds creating these components during every render is, but it’s probably negligible. I’m definitely looking forward to doing the initial HoC implementation. You can also reduce work during render by just creating the component once during lifecycle methods such as componentWillMount and componentWillReceiveProps, and then set them as state. So you have options here.
说的场景貌似不对,忽略。

@sbyps
Copy link

sbyps commented Nov 29, 2017

@WhatAKitty 代价可能还好说,就是动态加载的组件之前的数据state保存不下来,比如翻页等,这点我觉得是个问题。不过是否需要保存也是可以讨论的

@WhatAKitty
Copy link
Contributor Author

WhatAKitty commented Nov 29, 2017

@sbyps 数据的话,如果你使用了dva的话,那么按照dva的设计,数据是保存在model里面的,除非你自己将某些数据存储到了component级别。

编辑:
没看到你说的翻页。不过按照一般需求来说,翻页这些数据不必保存吧?如果真要保存,那么不如就直接存储数据到redux的state树内。

@sbyps
Copy link

sbyps commented Nov 29, 2017

@WhatAKitty 那也不是所有数据都在model里吧,我还是认为一些通用的数据放到model里好,一些就是组件内部用的还是保存在组件内部好一些,不然就是每个页面都要对应一个model了,这样很庞大也很多余吧

@WhatAKitty
Copy link
Contributor Author

@sbyps 担心这个的话,可以看这个dvajs/dva#77 ,这个问题是讨论过的。

@sbyps
Copy link

sbyps commented Nov 29, 2017

@WhatAKitty 还是认为再mount一次不是很合适,应该是recieveprops比较好,有什么解决思路吗

@WhatAKitty
Copy link
Contributor Author

WhatAKitty commented Nov 29, 2017

@sbyps
Copy link

sbyps commented Nov 30, 2017

@WhatAKitty 你这个关于state的思考我也很认同,但好像话题偏了=。= 在这个动态路由的情况下,组件是否还需要重新执行生命周期

@afc163
Copy link
Member

afc163 commented Nov 30, 2017

something maybe helpful remix-run/react-router#4578

@WhatAKitty
Copy link
Contributor Author

不小心发散开来了

@sbyps
Copy link

sbyps commented Nov 30, 2017

@afc163 在react-router的issue里面看了一圈,去除HOC并为routes设置相同的key能解决?我在本地试了一下好像还不行额。这个issue说的是path不同,同一个component不会被remount,但现在是不同的component,所以还是会被remount吧

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