Refactored type and property helpers #6721
Conversation
kraenhansen
added
T-Internal
no-changelog
no-jira-ticket
|
Putting this into draft as it seems I'm missing a few |
| PropertyHelpers, | ||
| PropertyOptions, | ||
| } from "./property-accessors/types"; | ||
| import { createIntPropertyAccessor } from "./property-accessors/Int"; |
There was a problem hiding this comment.
We could move this up with the other create imports 👍
| import { createDictionaryPropertyAccessor } from "./property-accessors/Dictionary"; | ||
| import { createDefaultPropertyAccessor } from "./property-accessors/default"; | ||
| import { createSetPropertyAccessor } from "./property-accessors/Set"; | ||
| import { createMixedPropertyAccessor } from "./property-accessors/Mixed"; | ||
| import { |
There was a problem hiding this comment.
Following the pattern in TypeHelpers.ts when there are many related imports in between.
| import { | |
| import { |
| function createUnsupportedTypeHelpers(): TypeHelpers { | ||
| return { | ||
| fromBinding() { | ||
| throw new Error("Not yet supported"); | ||
| }, | ||
| toBinding() { | ||
| throw new Error("Not yet supported"); | ||
| }, | ||
| }; | ||
| } |
There was a problem hiding this comment.
Nit: Would it make sense to have a type-helpers/unsupported.ts? 🤔
| }; | ||
| } | ||
|
|
||
| import type { PropertyAccessor, PropertyHelpers, PropertyOptions } from "./types"; |
There was a problem hiding this comment.
I think this accidentally got placed here instead of at the top.
| import { createUuidTypeHelpers } from "./type-helpers/Uuid"; | ||
| import { createArrayTypeHelpers } from "./type-helpers/Array"; | ||
|
|
||
| import type { TypeHelpers, TypeOptions } from "./type-helpers/types"; |
There was a problem hiding this comment.
type may be redundant for these imports. (If updating, there are more occurrences.)
There was a problem hiding this comment.
In general, I want to leverage the import type more to avoid circular dependencies at runtime (as these imports are stripped from JS files when running tsc). I still have an outstanding task of fixing these circular imports which got leaked into the developers bundling process, after we migrated away from Rollup.
That being said, the TypeScript compiler seems pretty good at shaking off these type-only imports, even if the type keyword isn't used.
There was a problem hiding this comment.
As you mention, I thought TS handled that implicitly when the import itself was a type as in this case. But are you saying that you prefer to still be explicit to be certain?
Co-authored-by: LJ <81748770+elle-j@users.noreply.github.com>
kraenhansen
force-pushed
the
kh/refactored-type-and-property-helpers
branch
from
70fd921
to
80e5d87
Compare
What, How & Why?
This refactors the type and property helpers into multiple files.
I want to move away from the "internal-pattern" to get rid of the circular reference warnings, which I somewhat started here and I'll have to do a separate (massive) PR later to complete that.
At some point, I think it'll also be nice to move some of the functions introduced in #6613 into type specific property helpers.