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

fix: correct defineConfig parameter typing #1389

Merged
merged 2 commits into from Dec 26, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 6 additions & 3 deletions src/types.ts
Expand Up @@ -7,10 +7,11 @@ import type { IModify } from '@umijs/core';
import type { AssetsPackage, ExampleBlockAsset } from 'dumi-assets-types';
import type { Element } from 'hast';
import type { IApi as IUmiApi } from 'umi';
import { defineConfig as defineUmiConfig } from 'umi';

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 并没有直接传给 umidefineConfig ,也确实不应该是 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

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 依赖,非常好用~ 😸


export interface IDumiConfig extends IUmiConfig {
interface IDumiExtendsConfig {
resolve: {
docDirs: (string | { type?: string; dir: string })[];
/**
Expand All @@ -32,9 +33,11 @@ export interface IDumiConfig extends IUmiConfig {
// eslint-disable-next-line @typescript-eslint/ban-types
extraRehypePlugins?: (string | Function | [string | Function, object])[];
}
export type IDumiConfig = IUmiConfig & IDumiExtendsConfig;

export interface IDumiUserConfig
extends Partial<Omit<IDumiConfig, 'resolve' | 'locales'>> {
extends Partial<Omit<IDumiConfig, 'resolve' | 'locales'>>,
IUmiConfig {
resolve?: Partial<IDumiConfig['resolve']>;
locales?: (
| IDumiConfig['locales'][0]
Expand Down
2 changes: 1 addition & 1 deletion tsconfig.json
Expand Up @@ -8,7 +8,7 @@
"baseUrl": "./",
"paths": {
"@/*": ["src/*"],
"@@/*": ["./.dumi/tmp"]
"@@/*": [".dumi/tmp/*"]
},
"types": ["vitest/globals"]
},
Expand Down