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: support --path with umi block #1444

Merged
merged 9 commits into from
Nov 19, 2018

Conversation

clock157
Copy link
Contributor

@clock157 clock157 commented Nov 12, 2018

  • 改写修改路由逻辑
  • 支持 layout path 参数

#1438

@coveralls
Copy link

coveralls commented Nov 12, 2018

Pull Request Test Coverage Report for Build 1975

  • 43 of 56 (76.79%) changed or added relevant lines in 3 files are covered.
  • 2 unchanged lines in 2 files lost coverage.
  • Overall coverage increased (+0.1%) to 33.542%

Changes Missing Coverage Covered Lines Changed/Added Lines %
packages/umi-build-dev/src/plugins/commands/block/index.js 0 3 0.0%
packages/umi-build-dev/src/utils/writeNewRoute.js 43 46 93.48%
packages/umi-build-dev/src/plugins/commands/block/block.js 0 7 0.0%
Files with Coverage Reduction New Missed Lines %
packages/umi-build-dev/src/plugins/commands/block/index.js 1 3.57%
packages/umi-build-dev/src/plugins/commands/block/block.js 1 18.57%
Totals Coverage Status
Change from base Build 1966: 0.1%
Covered Lines: 1159
Relevant Lines: 3515

💛 - Coveralls

蒲伟 added 3 commits November 13, 2018 21:39
@clock157
Copy link
Contributor Author

相关问题已改 @yutingzhao1991

@yutingzhao1991
Copy link
Contributor

关于 --layout-path 这个的逻辑还要想想。有几个问题:

  • 现在这个代码在加了 --layout-path 的情况下只对配置式路由生效,对于约定式路由其实地址还是 /${name},layout 也不没有,实际上这个参数是无效的。
  • 对于配置式路由,只是路由的配置改了,但是新的区块还是放在 pages 下面的,目录结构上没有体现出来。
  • 考虑要不要和 name 这个参数合并。

@yutingzhao1991
Copy link
Contributor

yutingzhao1991 commented Nov 14, 2018

我的想法是:

  • --layout-path--name 合并,改为 --path,默认就是当前的 /${name}
  • 文件存放的目录按照 --path 来,比如说 --path/test/admin 那么文件的目录结构就应该是 /test/admin/index.js。支持大小写。这样对于约定式路由自然 path 和文件夹就对应上了。
  • 对于配置式路由,文件的存放路径和约定式路由保持一致。然后把 --path 转换为小写,找到对应的路由配置插入进去。

关于配置式路由按照 --path(以 /test/admin/user 为例) 插入的逻辑如下:

  • 首先转换为小写,路径不区分大小写。
  • /test/admin/user 拆分为 / /test/test/admin,依次去路由里面查找匹配,找到了插入到对应节点的 routes (或者 childRoute 兼容 bigfish)下,否则和原有逻辑保持一致插入到最上面。

cc @sorrycc

@clock157
Copy link
Contributor Author

理解了,合并了可以简化一些,只是在 antd pro 中全是相对路径的情况要兼容下。
我稍后在 block.js 上再搞一波操作。

@clock157
Copy link
Contributor Author

@yutingzhao1991 已合并 layout 和 name 参数,支持正确的目录层级写入。

this.skipDependencies = opts.skipDependencies;
this.skipModifyRoutes = opts.skipModifyRoutes;
this.layoutPath = opts.layoutPath;
Copy link
Contributor

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.

是的,漏了这一个。

@clock157
Copy link
Contributor Author

现在有一个潜在问题,像 antd-pro 那种全是相对路径,如果存在同名二级路由,可能会匹配出错。
如 /users/settings 和 /email/settings,不过也可以解决,只是增加点代码,看要不要覆盖这种情况。

@yutingzhao1991 yutingzhao1991 changed the title refactor/write new route feat: support --path with umi block Nov 19, 2018
@yutingzhao1991
Copy link
Contributor

@clock157 路径相对和绝对的写法应该都是支持的,不应该影响应该有的逻辑。再单独提 PR 吧。

@yutingzhao1991 yutingzhao1991 merged commit 7829be8 into umijs:master Nov 19, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants