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

Jump to bottom

fix: correct defineConfig parameter typing #1389

Merged
merged 2 commits into from Dec 26, 2022
Merged

fix: correct defineConfig parameter typing #1389

merged 2 commits into from Dec 26, 2022

Conversation

tinymins
Copy link
Contributor

🤔 这个变动的性质是？/ What is the nature of this change?

  • 新特性提交 / New feature
  • bug 修复 / Fix bug
  • 样式优化 / Style optimization
  • 代码风格优化 / Code style optimization
  • 性能优化 / Performance optimization
  • 构建优化 / Build optimization
  • 网站、文档、Demo 改进 / Website, documentation, demo improvements
  • 重构代码或样式 / Refactor code or style
  • 测试相关 / Test related
  • 其他 / Other

🔗 相关 Issue / Related Issue

💡 需求背景和解决方案 / Background or solution

#1387

📝 更新日志 / Changelog

Language Changelog
🇺🇸 English fix defineConfig parameters typing check in .dumirc.ts file
🇨🇳 Chinese 修复 .dumirc.ts 文件中 defineConfig 函数传入的参数子项类型校验问题

@vercel
Copy link

vercel bot commented Dec 23, 2022 •
edited

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
dumi ✅ Ready (Inspect) Visit Preview Dec 23, 2022 at 10:04AM (UTC)

@tinymins
Copy link
Contributor Author

image

It works well now.

@tinymins tinymins force-pushed the fix-dumi-config-interface-typing branch from a9f165c to 94e7e76 Compare December 23, 2022 09:50
@tinymins tinymins force-pushed the fix-dumi-config-interface-typing branch from b4922a8 to 86aa9c5 Compare December 23, 2022 10:02
@tinymins
Copy link
Contributor Author

tinymins commented Dec 23, 2022 •
edited

image

It also supports autocomplete for umi config now!!! (due to docs in https://d.umijs.org/config)

image

PeachScript
PeachScript reviewed Dec 26, 2022
View reviewed changes
Copy link
Member

@PeachScript PeachScript left a comment •
edited

Choose a reason for hiding this comment

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

我初始化了一个新项目，按照 PR 修改 node_modules 下的产物发现还是找不到类型，原因应该是 umi/dist/types.d.ts 里解析不到 @@，看来模板里的 tsconfig.json 得加一下 include: ['.dumi/**/*'] 了，还有其他解法么

src/types.ts

type IUmiConfig = IUmiApi['config'];
type IUmiConfig = Parameters<typeof defineUmiConfig>[0];
Copy link
Member

Choose a reason for hiding this comment

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

用 ReturnType 更简单点？

Copy link
Contributor Author

Choose a reason for hiding this comment

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

我初始化了一个新项目，按照 PR 的改动发现还是找不到类型，原因应该是 umi/dist/types.d.ts 里解析不到 @@，看来模板里的 tsconfig.json 得加一下 include: ['.dumi/**/*'] 了，还有其他解法么

这个有解的，在 npm publish 之前用 tsc-alias -p tsconfig.json 处理一下就好了，原理就是在最后发布给用户之前去掉所有的 alias （将其替换为相对目录）。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

诶不是，等会，@@/* 不是必定在 tsconfig / alias 里面嘛， dumi 的教程里头有来着， https://d.umijs.org/guide/upgrading#%E6%9B%B4%E6%96%B0-tsconfigjson

Copy link
Member

Choose a reason for hiding this comment

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

换引入路径的方法可能不行，主要 Umi 项目的 @@ 指向位置不确定，加上 umi/dist/types.d.ts 也不能直接从 ../../../.dumi/tmp 里引入，这样 monorepo 提升依赖的场景可能就挂了

Copy link
Member

Choose a reason for hiding this comment

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

在最后发布给用户之前去掉所有的 alias （将其替换为相对目录）

题外话，顺便推荐下 father，编译时会根据 tsconfig.json 自动处理 paths 替换为相对路径，底层是基于 tsconfig-paths，以后有类似构建需求可以试试看😁

Copy link
Member

Choose a reason for hiding this comment

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

因为在我理解 TS 主要作用是描述一个数据从哪里来，要去往哪里，并不是只要类型刚好一样我们就可以拿来用

认同这个观点；defineConfig 不会被框架内部调用，它的返回值即配置文件 .dumirc.ts 最终的导出结果，也就是用户的声明值，框架内部会消费配置文件的导出结果

Copy link
Member

Choose a reason for hiding this comment

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

确实 我看了下我几天前的测试 dumi-demo 工程也改过 tsconfig

那看来绕不过了，我改下模板

Copy link
Contributor Author

Choose a reason for hiding this comment

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

defineConfig 不会被框架内部调用，它的返回值即配置文件 .dumirc.ts 最终的导出结果

站在用户使用的角度来看， .dumi.ts 中的 config 对象是 即将传递给 dumi 和 umi 的 config 的联合体，从这个角度来说，就应该是 Parameters<dumi.defineConfig>[0] & Parameters<umi.defineConfig>[0]，也是我最开始修改的思路。

但是站在 dumi 内部来看，这个 config 并没有直接传给 umi 的 defineConfig ，也确实不应该是 Parameters。

所以是否应该改成 ReturnType 我有点疑惑了。 😂

Copy link
Member

Choose a reason for hiding this comment

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

所以是否应该改成 ReturnType 我有点疑惑了。 😂

保持现状吧不纠结，dumi 的 defineConfig 入参继承自 Umi 的 defineConfig 入参，也说得通 👀

Copy link
Contributor Author

@tinymins tinymins Dec 26, 2022 •
edited

Choose a reason for hiding this comment

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

题外话，顺便推荐下 fath

Re: 题外话，已经升级至 v4 并且移除了 tsc-alias 依赖，非常好用~ 😸

PeachScript
PeachScript approved these changes Dec 26, 2022
View reviewed changes
Copy link
Member

@PeachScript PeachScript left a comment

Choose a reason for hiding this comment

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

+1

@PeachScript PeachScript merged commit 63fd21e into umijs:master Dec 26, 2022
@PeachScript PeachScript mentioned this pull request Jan 12, 2023
Closed
10 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@PeachScript PeachScript PeachScript approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@tinymins @PeachScript