Skip to content
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

V11 Lint improvements #2638

Merged
merged 3 commits into from
Jul 17, 2024
Merged

V11 Lint improvements #2638

merged 3 commits into from
Jul 17, 2024

Conversation

kmoschcau
Copy link
Collaborator

@kmoschcau kmoschcau commented Jul 14, 2024

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

Closes #2609.

@kmoschcau kmoschcau requested a review from a team as a code owner July 14, 2024 13:41
@kmoschcau kmoschcau enabled auto-merge July 14, 2024 13:43
@JPMeehan JPMeehan changed the base branch from main to v11/data-models July 14, 2024 16:39
@JPMeehan
Copy link
Collaborator

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
Copy link
Collaborator Author

What exactly do you mean by that?

@JPMeehan
Copy link
Collaborator

JPMeehan commented Jul 14, 2024 via email

@kmoschcau
Copy link
Collaborator Author

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
Copy link
Collaborator

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
Copy link
Collaborator Author

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

@kmoschcau kmoschcau merged commit b4d9d32 into v11/data-models Jul 17, 2024
2 of 6 checks passed
@kmoschcau kmoschcau deleted the v11/lint-improvements branch July 17, 2024 17:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Lint against .mjs in imports
2 participants