fix: fix overwrites pnpm-workspace.yaml and .yarnrc.yml into json #1589
Conversation
There was a problem hiding this comment.
renamed to upgradeJsonCatalogDependencies.ts
| { | ||
| "editor.defaultFormatter": "esbenp.prettier-vscode" | ||
| "editor.defaultFormatter": "esbenp.prettier-vscode", | ||
| "editor.formatOnSave": true |
| document: Document, | ||
| path: string[], | ||
| { newName, newValue }: { newName?: string; newValue?: string }, | ||
| ): Document | null { |
There was a problem hiding this comment.
Returning the Document and mutating it is a bit confusing. A pure function would be ideal. If that's not possible, then a mutation + returning a boolean success state would be a bit more clear.
We should also add a comment in the JSDOC that the document may be mutated even if the return state is false which appears to be possible if newName is valid and newValue is not a scalar.
| } | ||
|
|
||
| /** | ||
| * |
| type UpdateDependencyConfig = { | ||
| path: string[] // e.g., ['catalogs', 'my-catalog', 'my-dep'] or ['catalog', 'my-dep'] | ||
| newValue: string // e.g., '^2.0.0' | ||
| } |
There was a problem hiding this comment.
Let's inline this type as it is only used in one place, then we get the benefit of co-location.
| document = parseDocument(fileContent, { keepSourceTokens: true }) | ||
| parsedContents = CatalogsConfig.parse(document.toJSON()) | ||
| } catch (err) { | ||
| return null |
There was a problem hiding this comment.
We probably want to be a bit more specific here and avoid silently swallowing all errors. If there is a syntax error that prevents the file from being read, we should bubble that up to the user using programError. Otherwise they may assume that it was a successful run and there were no catalog upgrades.
3 participants
Refs: #1540