-
Notifications
You must be signed in to change notification settings - Fork 28
[code-infra] Increase build command type-safety #716
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
Conversation
Bundle size report
|
const importPath = originalExports[key]; | ||
if (!importPath) { | ||
set(newExports, [key], null); | ||
return; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe this should've been continue
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes.
* @param {string} param0.outExtension | ||
* @param {boolean} param0.addTypes | ||
* @returns {Promise<{path: string[], importPath: string | Record<string, string | undefined>}>} | ||
* @returns {Promise<void>} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some of the props were applied inside the function, some outside, and some both. Let's just update newExports
inside of the function to simplify the flow?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for adding the error messages. More robust now.
Extracted from #715
I'm currently messing with the exports code, to reduce the chance of regressions, adding extra types for package.json. Also removed lodash, it's a bit unnecessary,
set
breaks all type safety.I'm adding a few errors for patterns our exports translation doesn't support. Better to explicitly error to warn maintainers early. it also makes type checking work.
Docs infra package seems unaffected