fix: refactor CopyData interface to have the correct structure #7344
Conversation
There was a problem hiding this comment.
LGTM modulo comments below and some pedantry:
-
Do the first two commits (at least) not contain breaking changes? I note neither (nor the PR) is marked with "!" in the title nor "BREAKING CHANGE" in the description.
-
I think I'd have titled most of these commits, and the PR itself, as
refactor:rather thanchore:orfix:. -
Changes the
CopyDatainterface toICopyDataಠ_ಠ for the usual style guide reasons (but I realise I am not going to win this).
| @@ -22,15 +22,17 @@ export class BlockPaster implements IPaster<BlockCopyData, BlockSvg> { | |||
| ): BlockSvg | null { | |||
| if (!workspace.isCapacityAvailable(copyData.typeCounts!)) return null; | |||
|
|
|||
| const state = copyData.saveInfo as State; | |||
There was a problem hiding this comment.
Curiosity: are you removing this just because it isn't a very useful temporary binding, or is there a type-checking reason?
There was a problem hiding this comment.
The saveInfo used to be typed as Object | Element because blocks and workspace comments were using the same interface, which forced me to cast. But now they are using separate interfaces with proper types, so I could remove the cast =)
The
The only approved ones are
Haha I agree man. But consistency 🤷 |
Huh. TIL. I've always worked from the Conventional Commits spec. |
The basics
npm run formatandnpm run lintThe details
Resolves
Work on #7337
Proposed Changes
Changes the
CopyDatainterface toICopyDataand changes it to only require thepaster: string;property.Updates everything using this type/downstream APIs to conform to the new interface.
Reason for Changes
Working toward more flexible copy pasting.
Test Coverage
Should be covered by existing tests!
Documentation
N/A
Additional Information
Best to review commit-wise due to renames =)