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

API增加忽略属性功能的实现方案 #1644

Open
windyrain opened this issue May 6, 2023 · 8 comments
Open

API增加忽略属性功能的实现方案 #1644

windyrain opened this issue May 6, 2023 · 8 comments
Labels
discuss Something need to discuss help wanted Extra attention is needed

Comments

@windyrain
Copy link
Contributor

问题

因为我在使用 dumi 搭建团队的组件库文档,组件库内有一部分组件是包裹式组件,利用 React.cloneElement 去克隆子节点,然后完成功能,所以导致父组件,需要继承 HTMLElement 的属性。而自动生成会携带大量的属性,这对于使用者来说是不可接受的。

interface A extends React.DetailedHTMLProps<React.HTMLAttributes<HTMLDivElement>, HTMLDivElement> {
    a: string;
}

image

尝试的方案

我尝试对这种场景进行修改,一开始我想给 API 增加一个 ignoreattrs 属性,但 md 文件似乎不支持数组属性。会直接解析成文本。

image

所以我只能将这个属性改成字符串属性,用,分割,因为我要忽略大量的属性,所以不想在 md 中直接写入属性名,那样会导致md文件可读性变差,所以我声明了一个替换符号,名为 HTMLATTRS,整体效果如下

<API id="Foo" ignoreattrs="HTMLATTRS"></API>

然后我增加了一个 rehypeIngoreAttrs.ts 的插件,插件传入 extraRehypePlugins 配置中,作用是替换 md 文件中的简写字符串
实现如下:

image

然后我修改了 builtins/API 的代码,在表格渲染中对忽略的属性进行了过滤

image

疑惑

  1. 我不太清楚 transform/markdown 的执行时机,从目前的情况来看,他只在我变动 md 文件的时候才会执行,所以我不能确保上述方案能否正常使用。而且启动时会出现属性解析中请稍后的提示,等待属性解析完成后,页面会惹更新,也没有触发 markdown 的解析。
  2. 在写 rehypeIngoreAttrs 插件时,我注意到 node.properties 是一个 Record<string, string | number | (string | number)[] ......> 属性的,但是设置数组的值会被自动转换为空格分割的字符串,导致 builtins/API 的实现不够优雅,忽略属性名需要 split(',')
@windyrain windyrain added the question Further information is requested label May 6, 2023
@PeachScript
Copy link
Member

PeachScript commented May 6, 2023

感谢发起讨论并提供方案,这的确是个急需解决的问题👍

不过描述中的思路不是很建议,目前有 3 种思路,按推荐程度排序:

  1. 解析器(parser)支持过滤参数:但由于解析器没有开源,虽然是最佳方案,但执行进度强依赖我们内部相关团队的研发投入,坦白说目前是搁置状态
  2. 支持配置过滤条件,在解析后过滤(assets.ts):这是备选方案,可以先与 dumi 1 实现相同的配置项,后续 1 实现的情况下也可以考虑做适配,让用户没有感知
  3. 在 API 组件里过滤(运行时):这个方案不太建议,原因是过滤行为发生在运行时,这意味着构建产物里仍然包含大量开发者不需要用到的解析定义,它们多的时候甚至会有好几兆

所以按照目前的情况,如果你感兴趣且有精力参与 dumi 的话,比较建议第 2 种思路,你看看有没有其他的思路

@PeachScript PeachScript added help wanted Extra attention is needed discuss Something need to discuss and removed question Further information is requested labels May 6, 2023
@github-actions
Copy link

github-actions bot commented May 6, 2023

Hello @windyrain. We totally like your proposal/feedback, welcome to send us a Pull Request for it. Please be sure to fill in the default template in the Pull Request, provide changelog/documentation/test cases if needed and make sure CI passed, we will review it soon. We appreciate your effort in advance and looking forward to your contribution!

你好 @windyrain,我们完全同意你的提议/反馈,欢迎直接在此仓库创建一个 Pull Request 来解决这个问题。请务必填写 Pull Request 内的预设模板,提供改动所需相应的 changelog、测试用例、文档等,并确保 CI 通过,我们会尽快进行 Review,提前感谢和期待您的贡献。

@windyrain
Copy link
Contributor Author

@PeachScript 感谢你的建议,我准备从 assetParsers/atom.ts 入手改造,我想新加一个配置项,名为:overrideProperties,作用是覆盖生成好的 propsConfig 或者 signature 中的 properties 属性。相关代码,使用方式如下图:

image

image

逻辑基本上写好了,但是 umijs 似乎不允许增加额外的配置项,有这个报错,请问接下来我应该怎么做,有什么好的建议吗?

image

@PeachScript
Copy link
Member

我想新加一个配置项,名为:overrideProperties

建议先提供 dumi v1 的几个过滤有关的配置项:https://v1.d.umijs.org/zh-CN/config#apiparser
开放 override 灵活性和使用成本都比较高,框架尽量还是提供收敛性的配置项

但是 umijs 似乎不允许增加额外的配置项

这是因为 parser.ts 插件的 schema 没有做对应的修改,参考:https://d.umijs.org/plugin/api#describe

@windyrain
Copy link
Contributor Author

@PeachScript ,好的,我试试~

@windyrain
Copy link
Contributor Author

@PeachScript 我只加上了 skipPropsWithName 其他两个属性暂时没有好思路,判断 node_modules 和文档没有想到好的判断方式,你这有好的思路吗,我也可以加上。

@PeachScript
Copy link
Member

判断 node_modules

这个不好做,解析器没暴露相关信息,我想一下解法先

和文档

这个判断 title、description 应该就能满足?本意是只要用户写了 jsDoc 就意味着是有 API 文档的

@sdhr27
Copy link
Contributor

sdhr27 commented Sep 5, 2023

可以参考这个改一下 https://github.com/umijs/dumi/pull/807/files

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discuss Something need to discuss help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

3 participants