[BUG] namespace XXX {} is a slow and out-of-date syntax that typescript team itself moved away from
#408
Comments
|
@loynoir Heya, Tree Shaking Namespaces | IIFEI wouldn't expect any bundler to try and tree shake at the object level. Also the code example you have there is importing the Moving Namespaces to ModulesIn future, TypeBox may move each namespace to a ESM module (i.e. in a specific Namespaces Out of DateNamespaces are not out of date. They are merely aliases for IIFE's (as mentioned above). But as per quote (with relevant takeaways highlighted in bold)
And the TypeBox project "feels the need" to use them (at least for now) Namespace PerformanceThe suggestion here is that performance is improved for statically bound identifiers, where-as IIFE's evaluate return structures at runtime. This is true (as far as I've observed in V8 optimizations). However, the IIFE's being considered (presumably the TypeRegistry, FormatRegistry, TypeResolvers) here are called so infrequently, you're not likely to see any significant performance increase moving these to modules in real codebases (with perhaps a minor bump for the Resolvers which are called through type composition, but even these are hindered by dynamic object creation for the schematics) The main components targeted for performance is the Here is the current compilation benchmarks using IIFE. Link here If you can show a 30% improvement for compilation (or validation) performance, I'll consider a refactor of the compiler components into sub modules (under a different GH issue specific to performance). SummaryWhile replacing namespaces for modules are going to be better in the long term (and is planned for), this isn't something I'm prepared (or able) to implement at this time. As it stands.
Copy and paste the following into a <html>
<script type="module">
import { Type, TypeRegistry } from 'https://esm.sh/@sinclair/[email protected]'
console.log(Type) // ok
console.log(TypeRegistry) // ok
</script>
</html> |
|
@loynoir Going to close this one out. But would be open to a different issue related to performance if you can demonstrate If benchmarking your side, the Cheers |
If tree-shake shake out anything leads to different output, it must be a bug. Anything after optimization have a runtime difference, I wil use word
My main goal is to decrease bundle size. To achive that goal, source code should be written in good style of tree-shake think it is definitely safe. Thus, at prev feat, I suggest a way have good tree-shake support, using But current code style use I tested And thus, I open a feat in esbuild to tree-shake default export. That is why
Because I'm thinking about the PR implement
And I may implement in PR like below,
==> user.mts <==
import { someFn } from './typebox.mjs'
someFn()
==> typebox.mts <==
import * as TypeRegistry from './TypeRegistry.mjs'
function someFn() {
TypeRegistry.Clear()
}
export { someFn, TypeRegistry }
==> TypeRegistry.mts <==
const map = new Map<string, unknown>()
export function Entries() {
return new Map(map)
}
export function Clear() {
return map.clear()
}
// should not use, `tree-shake export default` is not supported by any bundler
// https://github.com/evanw/esbuild/issues/3077#issuecomment-1518713419
// export default { Entries, Clear }
$ npm exec -- esbuild --format=esm --bundle ./user.mts
// TypeRegistry.mts
var map = /* @__PURE__ */ new Map();
function Clear() {
return map.clear();
}
// typebox.mts
function someFn() {
Clear();
}
// user.mts
someFn();As typescript have a very large codebase, so can benifit a lot even from small optimization idea. I don't think typebox can benifit a lot, besides I feel enough about typebox performance, not my main goal. My main goal is possible bundle size So, the problem is are you willing to change, if only shows smaller size? If you are not willing to accept that PR only shows smaller bundle size, then I have no interest to implement such large PR when I have time. |
techical detail
evanw points out
summary
That is why
Again, I feel enough about current typebox performance, thus I lack to interest. But I feel not enough about bundle size. |
|
@loynoir Heya
In TypeBox, there has be a lot of care and attention given to ensuring module cross dependence is only taken when necessary. In this regard, TypeBox doesn't rely on tree-shaking, nor does it assume tree-shaking is even available in a users toolchain. Instead it is specifically crafted such that bundle sizes are proportional to the modules (or sub modules) a user directly imports, with each module considered to be an "un-shakable" unit due to cross coupling for things internal to any given module. This is a designed for, it's not an oversight. Of course, there is always ways to improve things. For example, currently the
Unfortunately, I can't accept a PR that splits the
This isn't a coincidence. TypeBox will become fully ESM closer to a 1.0 release, which each sub module split at the namespace level. This is planned for with the namespaces denoting where the module should be split (but not at this time)
I am very mindful of keeping bundle sizes down, but at this stage. It's just not a high priority for the project. For example this is the current breakdown of TB bundle sizes compared to the similar libraries. https://bundlephobia.com/package/@sinclair/[email protected]
The focus at this stage to keep bundle sizes down through using algorithm techniques, not through sub modules and reliance on tree shakers (those are bonus once TB finally transitions to ESM) |
Oh, I understand you. You mean you don't want current Beside,
In my proof PR, I do a simple two commit, got 12.7% smaller size.
Bundle size decrease from 148.7 to 129.8. Said about I feel the size can be better than 129.8, because Besides, I feel not enough about bundle size, but still acceptable. Is there a estimate time when typebox will move away from |
The TypeRegistry actually has cross dependence with the TypeGuard as it's used to verify a type is registered prior to compilation. Also as the TypeGuard is used extensively during type composition, so both are critical components of Line 1341 in 376e482
I'm not sure at this stage, but the following is a very high level (and tentative) series of steps that need to happen to remove them.
Each step is a minor revision in and of itself, leading towards the following import scheme. import TypeCompiler from 'typebox/compiler'
import Value from 'typebox/value'
import Type, { Static } from 'typebox'And optionally import { ValueCheck } from 'typebox/value' Initial estimates for namespace removal are likely 4th quarter 2023 (all things going well) or early 2024. Until then, priority is going into achieving all envisioning functional requirements while preserving the current namespace setup (as it has generally provided me most agility and flexibility to evolve the project over the years without breaking users or being locked into a finalized API design). Hope this helps! |
|
Sorry for forget to comment out const map = new Map<string, unknown>()
export function Entries() {
return new Map(map)
}
export function Clear() {
return map.clear()
}
// should not use, `tree-shake export default` is not supported by any bundler
// https://github.com/evanw/esbuild/issues/3077#issuecomment-1518713419
// export default { Entries, Clear }import * as TypeRegistry from './TypeRegistry.mjs'
function someFn() {
TypeRegistry.Clear()
}
export { someFn, TypeRegistry }Quote evanw
|
Description
evanw/esbuild#3077 (comment)
Evanw suggest syntax of
import * as TypeRegistry from './lib.mjs'.Which have is a dot property syntax, like namespace, with tree shake support.
evanw/esbuild#3077 (comment)
microsoft/TypeScript#30994 (comment)
microsoft/TypeScript#39247
microsoft/TypeScript#51387
Related
#401
#399
The text was updated successfully, but these errors were encountered: