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

Expose loadConfig to consumers #579

Closed
egorio opened this issue May 18, 2023 · 3 comments · Fixed by #606
Closed

Expose loadConfig to consumers #579

egorio opened this issue May 18, 2023 · 3 comments · Fixed by #606
Labels
enhancement New feature or request

Comments

@egorio
Copy link
Contributor

egorio commented May 18, 2023

I'm working on a tool to migrate JS to TS, including Glint project. The tool needs access to glint config values from tsconfig.json file and team is implementing a function to do this, but there are few challenges there to make it work right.

As you know, the standard TS functions to parse config ignore glint option, so you can't use them.
It's required to implement a function to search for this value recursively across all tsconfig hierarchy and merge all found glint values.

Glint has its own function to load the config to do this as well.

The basic implementation of the function to load config is not complex, but if we want to have it typed and validated then we need to "reimplement" the glint/packages/core/src/config/ which sounds counter-productive while glint has it already.

It's also a good practice to provide such functionality for open-source community, for example prettier's resolvaConfig or eslint's calculateConfigForFile.

So the question is could Glint has loadCondig and it's types (already exposed) to be exported to end user?

@egorio
Copy link
Contributor Author

egorio commented May 18, 2023

I can come with PR if Glint team will agree to export it.

@dfreeman
Copy link
Member

dfreeman commented May 19, 2023

It's also a good practice to provide such functionality for open-source community, for example prettier's resolvaConfig or eslint's calculateConfigForFile.

As a reminder, none of the programmatic interfaces exposed from @glint/core are officially public, and while we'll make a good-faith effort not to break those interfaces unnecessarily, they're intentionally undocumented and not a part of the semver contract for the package. Given that, comparing to ESLint and Prettier's public APIs isn't necessarily the best point of reference for "good practice" 😉

That said, since we already provide access to the GlintConfig instance to consumers who run analyzeProject, it seems reasonable to me to offer a way to load that config without forcing a full project analysis.

I can come with PR if Glint team will agree to export it.

I think exposing the loadConfig function that we already import and use in analyzeProject makes sense, and would be happy to review a PR to add that export.

@egorio
Copy link
Contributor Author

egorio commented Jul 31, 2023

@dfreeman here is the PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants