-
-
Notifications
You must be signed in to change notification settings - Fork 556
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
Maintenance strategy: Considering splitting type-fest
into smaller importable parts
#809
Comments
Could Note, lodash does this too. The options are: import {merge} from 'lodash'
import merge from 'lodash/merge'
import merge from 'lodash.merge'
import {merge} from 'lodash-es'
import merge from 'lodash-es/merge' Edit: I imaging the reason for lodash micro packages are to reduce install size - not so much of a concern for type-fest I think. So adding exports wouldn't prevent publishing micro packages in future. But micro packages sound like hell to manage, both for maintainers and users. I've asked renovate to update type-fest for umzug in the meantime! |
@mmkal Yes, that was my thinking, that export * from './source/primitive';
export * from './source/typed-array';
export * from './source/basic';
export * from './source/observable-like'; To: export * from '@type-fest/basic';
If TypeScript is parsing and validating lots more types than it needs to, then I guess it could have an impact on its performance, especially when Edit: I misread you, yeah pushing for using |
I think so too. type-fest is strongly correlated with TS version. It's not uncommon for mismatched versions to have problems. Either use the correct (recommended in the README) version, or use but "excessive import" (or why this type I think we should balance cost and how likely is such a problem to occur in the future, especially after adding That's all I can think of currently. |
I like the suggestion of using |
Also – unless we encourage packages to set
|
That can be solved by using named exports rather than default. Lodash use default, but type-fest could do: import {MergeDeep} from 'type-fest/merge-deep' That way vscode would offer both options when typing "MergeD..." and pressing cmd-space, without forcing users to know about and install hundreds of packages. |
Would it be much different from what it is today? Would there be a way to know that its an officially supported way to import? Today you also get suggestion to import it from a subpath, but hopefully people don't right now |
Yes, I think it'd be different, because being part of Adding to exports, combined with some docs, would be enough to hint that people can/should use it, I think. But you're right it's not much different from the status quo, especially in terms of how little implementation + maintenance work it is. But that's a good thing! |
One could potentially use something like https://github.com/timocov/dts-bundle-generator to bundle and publish those types from a totally separate repository, avoiding any added complexity here |
Publishing micro-packages for |
Why is TypeScript even parsing and validating unused types? I guess it's because there could be global declarations or I also wonder if moving to ESM will improve this. At least for JS, ES modules are lazily parsed, so you only pay for what's imported. |
type-fest
into multiple modulestype-fest
into smaller importable parts
Anyone up to open such an issue / investigate? @sindresorhus What's your view on I'm also pondering of this from the Small Focused Module point of view. The one focus of this module is |
I prefer to focus on the real fix. This is not a wide-spread problem anyway.
One of the main benefits of small focused modules is reducing runtime size, but that's not a concern for types as they are a compile-time construct. In this case, having everything in one package is the pragmatic choice, especially because many types depend on others, and having them in separate packages would add a lot of overhead in maintenance, install time, and usage. I still believe in small focused modules, but I think I have become more pragmatic about it throughout the years. For example, with is or uint8array-extras. |
I'll close this one then and we'll pick up the fix for TypeScript when/if anyone wants to pursue that :) |
The issue in #784, which got fixed in the highlights a larger problem with the
4.10.2
-release, is one that highlights a bigger challenge with the current scope of thetype-fest
package:It contains a lot of types, some which are more complex than others.
Eg.
MergeDeep
andCamelCase
are far more complex than eg.JsonValue
andPrimitive
are, yet if you importtype-fest
all types gets parsed and validated (unless you setskipLibCheck
, something that disables even.d.ts
checks within ones own project, see #785)Context
I run my own canary type tests on my own modules and eg. my
umzeption
project is still failing its type canary, despite the release of4.10.2
.Why? Well,
umzug
(a dependency of my module) relies ontype-fest@^3.0.0
(see sequelize/umzug#645) in two places:SetRequired
in one place andMergeExclusive
in another place – neither which relies onMergeDeep
, yet the canary tests of my modules fail due toMergeDeep
in3.x
not being compatible with TS 5.4.Solution
In a way
type-fest
is a "lodash for types" and just likelodash
moved to publish more granular packages in addition to their main package I think we should do something similar.Eg. following the README groups (should maybe be more granular as eg.
utilities
includes a lot):@type-fest/basic
@type-fest/utilities
@type-fest/type-guard
@type-fest/json
@type-fest/async
@type-fest/string
@type-fest/array
@type-fest/numeric
@type-fest/casing
@type-fest/misc
And probably also:
@type-fest/core
– core internal utilities that's relied on between multiple of above packagestype-fest
– as today, but importing the individual modules rather than containing all the code itselfI'm not a huge fan of mono-repos itself but I think it would serve a good purpose here and allow for a package like
umzug
to rely ontype-fest
without exposing themselves to all the possible future incompatibilities that the complex types may cause in the future.Thoughts @sindresorhus, @skarab42, @Emiyaaaaa, @BendingBender, @CvX and others?
Upvote & Fund
The text was updated successfully, but these errors were encountered: