search

League-of-Foundry-Developers / foundry-vtt-types

Unofficial type declarations for the Foundry Virtual Tabletop API
MIT License
116 stars 56 forks source link

Linting, Roll Migration #2774

Closed JPMeehan closed 2 months ago

JPMeehan commented 2 months ago

Sidebar: Should we just... delete DataConfig and SourceConfig since those are from the v8/v9 configuration setup, pre-data models?

LukeAbby commented 2 months ago

In theory DataConfig and SourceConfig still serve a purpose for systems that have a template.json but go without a data model.

We could just roll it into DataModelConfig but DataConfig and SourceConfig are backwards compatible... but I'm not sure how valuable that is anymore.

JPMeehan commented 2 months ago

In theory DataConfig and SourceConfig still serve a purpose for systems that have a template.json but go without a data model.

We could just roll it into DataModelConfig but DataConfig and SourceConfig are backwards compatible... but I'm not sure how valuable that is anymore.

Currently, TypeDataField doesn't leverage those two setups. My inclination is that this project should focus on supporting Foundry's intended runtime validation, data models.

LukeAbby commented 2 months ago

As far as I understand it, TypeDataField will never leverage those two because TypeDataField is inherently for when a system actually sets up a data model. This is for the case where they have a template.json but not data models.

It honestly doesn't add much complexity to keep supporting. Some systems don't want to use data models, I say let them. If it were more complex I'd possibly be swayed by the argument that we're trying to maintain high quality and robustly typed code anyways but it's about as complex as adding something like DataConfig extends { readonly [_ in DocumentName]: { [_ in DocumentType]: infer DataModel } } ? DataModel : EmptyObject or something similar.

JPMeehan commented 2 months ago

TypeDataField is where the template.json gets used - see getInitialValue and _cleanType. On the server backend, where community packages can't register data models, they're functionally identical,

Anyways, if we want to keep supporting those I'm fine with that but we'll need to update the definition of the field to re-use DataConfig/SourceConfig. We may also want to double check how those are implemented because of the structural changes for documents.