-
Notifications
You must be signed in to change notification settings - Fork 392
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
Remove schema namespace #3699
Remove schema namespace #3699
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Ignored Deployment
|
packages/@sanity/types/src/schema/definition/schema-definition.ts
Outdated
Show resolved
Hide resolved
packages/@sanity/types/src/schema/definition/type/cross-dataset-reference.ts
Outdated
Show resolved
Hide resolved
sortable?: boolean | ||
modal?: {type?: 'dialog' | 'popover'; width?: number | 'auto'} | ||
} | ||
options?: ArrayOptions<V> & {layout?: V extends string ? 'tag' : 'grid'} |
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.
Had to diverge from the definition type here, and narrow layout a bit. This is more correct, but supporting it in the definition is not worth it (need to drill the type of array.of every where via generics)
@@ -0,0 +1,131 @@ | |||
import {PreviewConfig} from '../preview' |
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.
Hm, actually – this file is already in a directory called definition
. Should this file be renamed to types.ts
then?
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.
I have no strong opinion on it.
packages/@sanity/types/src/schema/definition/schemaDefinition.ts
Outdated
Show resolved
Hide resolved
/** | ||
* A union of all intrinsic types allowed natively in the schema. | ||
*/ | ||
export type TypeName = IntrinsicTypeDefinition[keyof IntrinsicTypeDefinition]['type'] |
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.
IntrinsicDefinitionTypeName
?
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.
I would really like to keep this short, as it is used in a lot of generic contexts. If it is a long name, that reads worse imo.
IntrinsicTypeName perhaps?
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.
I think IntrinsicTypeName
makes sense, since it's built from keys of IntrinsicTypeDefinition
.
packages/@sanity/types/src/schema/definition/schemaDefinition.ts
Outdated
Show resolved
Hide resolved
packages/@sanity/types/src/schema/definition/schemaDefinition.ts
Outdated
Show resolved
Hide resolved
…initions This fixes naming conflict between namespace and interface, and will make definition type extensions easier to work with (by declaration merging interfaces in sanity module directly). Option-definitions is now shared between SchemaTypes and Definition types. BREAKING CHANGE: Code that accessed types in the Schema namespace must import the types directly instead.
b155c03
to
eaa0923
Compare
Had to recreate the branch since v3 branch was force-pushed or something. |
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.
LGTM! Namespaces be gone!
…to IntrinsicTypes
Description
Replaces the
Schema
namespace with plain exports.The contents of the namespace is split into file-per-type in /definitions.
Apart from namespace being gone, this only moves types around; Option types are now located with each type and reused by SchemaTypes where possible.
No attempt was made to improve the SchemaType typings beyond what they are today. This PR is about improving the definition types.
What to review
Does the code-organization look ok?
Notes for release
Schema
namespace has been removed. The same types can now be accessed as regular imports from thesanity
package.IntrinsicTypeDefinition
was renamed toIntrinsicDefinitions
. Module extensions relying on declaration merging to this type must rename the interface.