feat(psl-core): implement parse_configuration_multi_file to correct prisma/prisma's getConfig implementation
#4825
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
… TypeScript's "getConfig" function
jkomyno
changed the title
prisma/prisma's getConfig implementationparse_configuration_multi_file to correct prisma/prisma's getConfig implementation
jkomyno
commented
Apr 11, 2024
Comment on lines
+148
to
+153
| let asts = Files::new(files, &mut diagnostics); | ||
|
|
||
| for (_, _, _, ast) in asts.iter() { | ||
| let out = validate_configuration(ast, &mut diagnostics, connectors); | ||
| configuration.extend(out); | ||
| } |
There was a problem hiding this comment.
I purposedly avoided creating a ParserDatabase instance here, as it performs additional validations that are not retro-compatible with parse_configuration, used in prisma_fmt::get_config.
WASM Query Engine file Size
|
CodSpeed Performance ReportMerging #4825 will not alter performanceComparing Summary
|
|
Using
Before this PR, the output was: With this PR, the output is: |
jkomyno
requested review from
Druue and
SevInf
and removed request for
a team and
Druue
|
I confirm that applying these changes to |
✅ WASM query-engine performance won't change substantially (1.014x)Full benchmark reportAfter changes in 48f2d45 |
SevInf
reviewed
Apr 11, 2024
| } | ||
|
|
||
| /// Iterate all parsed files. | ||
| #[allow(clippy::should_implement_trait)] |
There was a problem hiding this comment.
Why not implement trait instead?
There was a problem hiding this comment.
Because of
`impl Trait` in associated types is unstable
see issue #63063 <https://github.com/rust-lang/rust/issues/63063> for more information
add `#![feature(impl_trait_in_assoc_type)]` to the crate attributes to enable
which appears when trying to write e.g.
impl std::iter::IntoIterator for Files {
type Item = (FileId, String, schema_ast::SourceFile, ast::SchemaAst);
type IntoIter = impl Iterator<Item = (FileId, String, schema_ast::SourceFile, ast::SchemaAst)>;
fn into_iter(self) -> Self::IntoIter {
self.0
.into_iter()
.enumerate()
.map(|(idx, (path, contents, ast))| (FileId(idx as u32), path, contents, ast))
}
}Also, iter and into_iter were previously defined like this by Tom, and I'd assume the reason he didn't implement the traits is the same one as mine. Do you have any explicit recommendation to work around this, or is it ok to leave the clippy warning suppressors as committed?
There was a problem hiding this comment.
Ah, i see. LGTM with no comments then
SevInf
approved these changes
Apr 11, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR closes https://github.com/prisma/team-orm/issues/1103.
This PR changes the implementation of
prisma_fmt::get_config, to actually support multiple schema files without validating them entirely. This fixes a non-retrocompatible regression introduced in #4787 and discovered in prisma/prisma#23824 (specifically, in DbPull.test.ts)./integration