-
Notifications
You must be signed in to change notification settings - Fork 35
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
feat(core): add configuration validation using typia #730
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,48 @@ | ||
import color from 'picocolors'; | ||
import * as typia from 'typia'; | ||
import type { RslibConfig } from './types'; | ||
|
||
export const validateConfig: ( | ||
input: unknown, | ||
) => typia.IValidation<RslibConfig> = typia.createValidateEquals<RslibConfig>(); | ||
Check failure on line 7 in packages/core/src/validate.ts
|
||
|
||
export function validate(input: unknown, configPath?: string): RslibConfig { | ||
const result = validateConfig(input); | ||
|
||
if (result.success) { | ||
return result.data; | ||
} | ||
|
||
const messages = result.errors.flatMap(({ expected, path, value }) => { | ||
if (expected === 'undefined') { | ||
// Unknown properties | ||
return [`Unknown property: \`${color.red(path)}\` in configuration`, '']; | ||
} | ||
|
||
return [ | ||
`Invalid config on \`${color.red(path)}\`.`, | ||
` - Expect to be ${color.green(expected)}`, | ||
` - Got: ${color.red(whatIs(value))}`, | ||
'', | ||
]; | ||
}); | ||
|
||
// We use `Array.isArray` outside to deal with error messages | ||
throw new Error( | ||
[ | ||
`Invalid configuration${ | ||
configPath ? ` loaded from ${color.dim(configPath)}` : '.' | ||
}`, | ||
'', | ||
] | ||
.concat(messages) | ||
.join('\n'), | ||
); | ||
} | ||
|
||
function whatIs(value: unknown): string { | ||
return Object.prototype.toString | ||
.call(value) | ||
.replace(/^\[object\s+([a-z]+)\]$/i, '$1') | ||
.toLowerCase(); | ||
} |
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.
typia
has so many dependencies, this is a bit hard to accept...https://pkg-graph.info/typia
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.
It indeed has too many dependencies, but it is tree-shakable and we are using it as
devDependencies
.The bundle size of Rslib is increased from 106.8 kB to 2484.4 kB. But the size change mainly produced by the validation functions generated by
typia
:As you can see, the
src/validate.ts
has 2.33 MB in stats. And we can make validation opt-in and lazy loaded.For end users, no additional dependencies need to be installed, and the bundle size of
@rslib/core
increases by only approximately 2.5 MB.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.
If we make the validation opt-in, how can users enable the validation?
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.
E.g.: if the
TypiaRspackPlugin
(which transformssrc/validation.ts
) is commented out, the bundle size of@rslib/core
in this PR is only 108.4 kB.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.
Maybe through a environment variable?
Or maybe a
strictDefineConfig
API?But I would prefer enabling by default. And opt-out using environment variables.
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.
If it's opt-in, I don't think most users will actively enable it...
If it's opt-out, the performance overhead doesn't seem worth it, we can write code to do some validation manually.
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.
Sure, I'm closing this PR. Thank you for your review