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

Allow specifying type definition files from .luaurc #576

Closed
wants to merge 9 commits into from
Closed

Allow specifying type definition files from .luaurc #576

wants to merge 9 commits into from

Conversation

Gargafield
Copy link

Implements #418

Right now, parsing the config checks the file existence, but this won't always work since the frontend file resolver might not work like the standard CLIFileResolver.

@asajeffrey
Copy link
Collaborator

Thanks for the contribution! Some questions...

  • Can more than one type definition file be loaded?
  • Are type definition files loaded relative to the luaurc file?
  • Can we have a unit test that ensures the decorations are in scope?

@zeux
Copy link
Collaborator

zeux commented Jul 5, 2022

There's a few caveats on the mechanical part of the change besides the above, most notably that Config code should not perform file system access, and Luau should never use anything from std::filesystem as this causes compat issues on earlier versions on macOS. We'd need to have that addressed -- however, the other issue is that we actually have not stabilized the type definition syntax yet, it only exists as an implementation detail.

As such I think we'd need an RFC that specifies the type definitions fully before being able to merge any externally visible support for type definitions, whether it's via command line or luaurc. cc @AmaranthineCodices for additional comments/concerns if any, as it's been a while since we talked about type defns internally.

@Gargafield
Copy link
Author

Gargafield commented Jul 5, 2022

  • Are type definition files loaded relative to the luaurc file?

This would require a more substantial implementation, the file path isn't currently carried across the config.

  • most notably that Config code should not perform file system access, and Luau should never use anything from std::filesystem as this causes compat issues on earlier versions on macOS.

Will find another way tomorrow.

  • the other issue is that we actually have not stabilized the type definition syntax yet, it only exists as an implementation detail.

We don't have to support the type definition syntax for now. Only allowing type exports should be fine.

@Gargafield
Copy link
Author

I'm unsure how we should tell the user about missing global types files. Either tell them in the config or do it while loading the module types. The former isn't viable and I'm not sure where the error should be for the latter. Do the maintainers have an opinion on this?

@Gargafield
Copy link
Author

Anymore questions or concerns on this? @asajeffrey @zeux

@Gargafield
Copy link
Author

I think I'm going to rename this feature to just typePaths instead of globalTypePaths. This is because globalTypePaths is misleading since the module will use the type paths of the nearest config.

@Gargafield
Copy link
Author

I think I will close this for now. Maybe I will open it again in the future when it gets more attention.

@Gargafield Gargafield closed this Sep 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants