refactor(cli): resolve TODO comment in Version.test.ts #18459
Conversation
ae8bc73
to
ee5e16f
Compare
CodSpeed Performance ReportMerging #18459 Summary
Benchmarks breakdown
|
| @@ -112,17 +112,15 @@ function cleanSnapshot(str: string, versionOverride?: string): string { | |||
| // ^^^^^^^^^^^^^^^^^^^ | |||
| str = str.replace(/\(at (.*engines)(\/|\\)/g, '(at sanitized_path/') | |||
|
|
|||
| // TODO: replace '[a-z0-9]{40}' with 'ENGINE_VERSION'. | |||
| // Currently, the engine version of @prisma/prisma-fmt-wasm isn't necessarily the same as the enginesVersion | |||
There was a problem hiding this comment.
cc @jkomyno do you know why they aren't necessarily the same versions?
There was a problem hiding this comment.
Technically one could use a different one if there are not relevant changes that affect prisma-fmt/schema-wasm maybe, but I don't think our setup is designed to ever actually do that. They should be in sync. (cc @millsp)
There was a problem hiding this comment.
You can safely remove that TODO.
Originally the Wasm's package version() method returned a constant string (with gibberish, and not the actual engines version). So, we started reading @prisma/prisma-fmt-wasm's version from its own package.json file, which was however upgraded less often than the rest of our Rust crates that result in @prisma/engines-version updates. At some point, we aligned the Rust pipeline with Wasm's, which made the comment highlighted above silently deprecated.
(I had originally missed this notification 🙈 @millsp)
|
Do you remember was the outcome of our conversation for this? |
|
|
This came up during a deep dive meeting with @millsp and @SevInf.
PR marked as draft because we were not sure.