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: declaration generation error (#733) #743

Closed
wants to merge 6 commits into from

Conversation

ninesunsabiu
Copy link

修改了 dts 的生成逻辑,放宽了 output 的约束,不再限制在 inputFiles 之中,因为类型的变更可能会影响多个文件

由于修改了 dts 生成逻辑,所以影响了 bundless 处写入文件的逻辑:忽略掉无法计算出 dist 输出目录的声明文件

Copy link

codecov bot commented Feb 25, 2024

Codecov Report

Attention: Patch coverage is 97.56098% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 94.53%. Comparing base (1d6debf) to head (bd02a64).

❗ Current head bd02a64 differs from pull request most recent head bd65648. Consider uploading reports for the commit bd65648 to get more accurate results

Files Patch % Lines
src/builder/bundless/index.ts 95.45% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #743      +/-   ##
==========================================
+ Coverage   94.45%   94.53%   +0.07%     
==========================================
  Files          55       55              
  Lines        1570     1591      +21     
  Branches      358      362       +4     
==========================================
+ Hits         1483     1504      +21     
  Misses         87       87              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ninesunsabiu
Copy link
Author

@PeachScript 烦请 review 这个 PR,看看是否可以合并?

@PeachScript
Copy link
Member

感谢贡献,看了下 PR 提供的方案有些复杂,希望 dts 生成尽量在自己的模块里处理完整,外部只关心写文件即可

想到个思路你看看是否可行:outputFiles 的过滤来源从 inputFiles 改成 inputFiles + cache,也就是说只要该文件变更引起的关联文件此前已经编译过了,那么就判定为有效输出

比如有 a.ts、b.ts 两个文件,初次编译时两者都会作为 inputFiles 传入,然后同时存入 cache,此时 a.ts 文件变更影响了 b.d.ts,通过判断 inputFiles (a.ts) 和 cache (b.ts),就能同时把 a.d.tsb.d.ts 纳入 outputFiles 一起输出

@ninesunsabiu
Copy link
Author

利用初次编译的 cache 来确定有效输出文件的范围,确实会更好点。当前方案最初确实是以 仅修改 dts 的 output 为目标,只是没想到兜不住范围泄漏到了 bundless/index 上

我晚些时候按照这个思路修改一下

感谢指出 🙏

@ninesunsabiu
Copy link
Author

感谢贡献,看了下 PR 提供的方案有些复杂,希望 dts 生成尽量在自己的模块里处理完整,外部只关心写文件即可

想到个思路你看看是否可行:outputFiles 的过滤来源从 inputFiles 改成 inputFiles + cache,也就是说只要该文件变更引起的关联文件此前已经编译过了,那么就判定为有效输出

比如有 a.ts、b.ts 两个文件,初次编译时两者都会作为 inputFiles 传入,然后同时存入 cache,此时 a.ts 文件变更影响了 b.d.ts,通过判断 inputFiles (a.ts) 和 cache (b.ts),就能同时把 a.d.tsb.d.ts 纳入 outputFiles 一起输出

在 dts 模块内 能给将 a.d.tsb.d.ts 作为 outputFiles 输出了,但是 在 bundless/index 里面 writeFileSync 中依然得不到应有的输出目录 (/dist/cjs or /dist/esm)

// prepare for declaration
if (result.options.declaration) {
// use winPath because ts compiler will convert to posix path
declarationFileMap.set(winPath(itemAbsPath), parentPath);
}

declarations.forEach((item) => {
fs.writeFileSync(
path.join(declarationFileMap.get(item.sourceFile)!, item.file),
item.content,
'utf-8',
);
});

这样就回到了如何得到输出的 parentPath 问题。 我之前在 bundless/index 脏了一把,也是为了处理这个问题。这个要怎么办呢?我之前的做法就是抽离出一个 itemPathInfo 的逻辑,用来得到 parentPath

@ninesunsabiu ninesunsabiu changed the title fix: declaration generation error (#733) fix: declaration generation error (#733) WIP Feb 29, 2024
@ninesunsabiu
Copy link
Author

ninesunsabiu commented Feb 29, 2024

过程中还发现 dts 的 use cache first 一个问题

image

针对这个想到一个方案:再做一个 inputFiles -> output 的 缓存值,然后 use cache first 的时候 读取的是 get cache by inputFiles,来避免 ts 增量缓存跳过 writeFile 过程而导致的 output 缺失.

发散的想, cjs/esm 中关于 dts 的生成 是不是应该一致的?所以,dts 的生成是不是可以独立于 bundless ,得到输出结果之后,再按需分配到 cjs/esm 的 dist 中,这样可以只执行一次 getDeclarations 而不用各自执行?

看到代码中关于 getDeclarations 的输入,是根据 runLoaders 的结果 result.options.declaration。这表示 dts 的生成输入还需要依靠 loaders 的结果,这一点依赖我不是很明白

在 cjs/esm 同时存在的情况下,会因为 ts 的增量编译而跳过
如果仅用 inputFiles 作为缓存初始值 可能存在某个 bundless 输出不全的问题
@ninesunsabiu ninesunsabiu changed the title fix: declaration generation error (#733) WIP fix: declaration generation error (#733) Feb 29, 2024
@ninesunsabiu ninesunsabiu changed the title fix: declaration generation error (#733) fix: declaration generation error (#733) WIP Mar 7, 2024
因为声明文件的输出范围可能没有经过输入文件的范围,只有输入文件范围才做过目录的创建
所以对于那些不在 declarationFileMap 中的文件 再次做一次 ensureDirSync
@ninesunsabiu ninesunsabiu changed the title fix: declaration generation error (#733) WIP fix: declaration generation error (#733) Mar 12, 2024
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.

2 participants