search

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

Unofficial type declarations for the Foundry Virtual Tabletop API
MIT License
111 stars 52 forks source link

V11 Lint improvements #2638

Closed kmoschcau closed 1 month ago

kmoschcau commented 1 month ago

This adds some linting improvements and also fixes things. Sadly it also uncovers more typecheck errors.

Closes #2609.

JPMeehan commented 1 month ago

Since we're updating import paths here — can we clean up DataModel? I know their implementation of the file is a mess with the exports, but we should clean that up for our purposes.

kmoschcau commented 1 month ago

What exactly do you mean by that?

JPMeehan commented 1 month ago

Look at the import/export statements in the DataModel file, both in our repo and in Foundry. It's exported at least twice and only the non-default export includes the namespace.

On Sun, Jul 14, 2024, 11:24 Kai Moschcau @.***> wrote:

What exactly do you mean by that?

— Reply to this email directly, view it on GitHub https://github.com/League-of-Foundry-Developers/foundry-vtt-types/pull/2638#issuecomment-2227443077, or unsubscribe https://github.com/notifications/unsubscribe-auth/AD6Y5AG5C3JI6VP2H7C3IOTZMK64NAVCNFSM6AAAAABK3GAZECVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDEMRXGQ2DGMBXG4 . You are receiving this because you commented.Message ID: <League-of-Foundry-Developers/foundry-vtt-types/pull/2638/c2227443077@ github.com>

kmoschcau commented 1 month ago

I can have a look later, but I thought this is modeling what foundry does. The exports, both named and default, should be the same as they are in core. And I thought if it's named the same, namespaces are exported either way. If they are not, I doubt there is a way to also export them as default.

JPMeehan commented 1 month ago

I can have a look later, but I thought this is modeling what foundry does. The exports, both named and default, should be the same as they are in core. And I thought if it's named the same, namespaces are exported either way. If they are not, I doubt there is a way to also export them as default.

While Foundry is internally a mess in this file, I think our project would be better served deviating from it — I should be able to go foundry.abstract.DataModel and then access any namespace properties.

The change that needs to happen is the export default class DataModel needs to be broken up into declare class DataModel and then export default DataModel

kmoschcau commented 1 month ago

I finally had the opportunity to have a look. Now I got what you mean and I changed it.