refactor: replace subcommands with arguments #36
Conversation
xDivisionByZerox
added
the
c: refactor
|
This implementation might be insufficient if Faker decides to implement functions with the same name (currently for example npx faker -m string hexadecimal
> 0xC
npx faker --module string hexadecimal
> 0xFIt is also noteworthy that the function name currently is matched case sensitive. This behavior might change as well in the future if it turns out to be bad DX. |
There was a problem hiding this comment.
For future descriptions please include a execution diff.
e.g.
// Old
faker [options] module_name entry_name
// New
faker int ....I see why you want to omit the sub commands.
But where is the actual implementation of the sub commands/ calls of the faker invocation?
Instead of using faker int we could also use faker number.int that way we have the module name directly in the command and don't have to randomly search the method, but we can instead directly refer to/resolve it. (I hope this translates well in v9 to an import e.g. import fn from @faker-js/faker/modules/number/int)
I hope that I wouln't have to change the interface to often. But can do if it should happen again.
Its here: Line 57 in 1483555 fn is the actual entry in the faker instance.
Yeah, I'm more looking forward to a tree-shakeable Faker that exprot all it's functions separatly. Thats why I decided to implement it with a single name argument. Additionally, I would not say that the format of |
Co-authored-by: ST-DDT <ST-DDT@gmx.de>
I thought there was already a way to provide parameters, but I'm mistaken. |
My concern is that just functionName might not be stable, because due to packaging or whatever magic (minification/variable aliasing), the modules could be in different order and thus Also if the dot notation is the main issue, isn't it possible to just add another argument like this: .argument(
'<module>',
'The name of the module to invoke.',
)
.argument(
'<function>',
'The name of the function to invoke.',
)
.action(async (moduleName, functionName) => { ... }) |
I liked that idea. So simple, that I didn't even consider it. Thank you for the suggestion 👍 |
xDivisionByZerox
changed the title
xDivisionByZerox
deleted the
refactor/replace-subcommands-with-argument
branch
Description
This PR significantly reduces the runtime duration of the CLI. This is achieved by using a single CLI argument instead of subcommands. While subcommands might be nicer for user experience, they are quite slow in execution. The next section contains some numbers on performance improvements.
Performance Numbers
*100 iterations per implementation
This equates to the following performance increases:
All Iterations
System Info: