Details
This stack is tested altogether, CI will fail at this point due to Flow breaking changes which will be solved in subsequent diffs.
Actual test plan in the last diff.
Diff Detail
- Repository
- rCOMM Comm
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
| lib/types/media-types.js | ||
|---|---|---|
| 689 ↗ | (On Diff #28255) | Is this by mistake? It does not match the type |
| lib/types/media-types.js | ||
|---|---|---|
| 689 ↗ | (On Diff #28255) | Yes, good catch. Leftover after experiments |
| lib/types/media-types.js | ||
|---|---|---|
| 680 ↗ | (On Diff #28255) | Same here as below |
| 688 ↗ | (On Diff #28255) | Not sure there's a point to wrapping this in tShape. You could just declare an object, and skip the .meta.props extraction below |
| 697 ↗ | (On Diff #28255) | Not sure what this export is for... I can't find any usages outside this file |
| 735 ↗ | (On Diff #28255) | I'm not sure you can spread a union type. I think this will lead to issues. You should reverse the approach and define the shared parts first: type EncryptedVideoShared = { +id: string, +encryptionKey: string, +type: 'encrypted_video', +dimensions: Dimensions, +loop?: boolean, +thumbnailID: string, +thumbnailEncryptionKey: string, +thumbnailThumbHash: ?string, }; export type EncryptedVideo = | { ...EncryptedVideoShared, +holder: string, +thumbnailHolder: string } | { ...EncryptedVideoShared, +blobURI: string, +thumbnailBlobURI: string }; |
| lib/types/media-types.js | ||
|---|---|---|
| 688 ↗ | (On Diff #28255) | Omitting this wasn't working in my initial solution but looks like it's working now, I'll do it |
| 697 ↗ | (On Diff #28255) | It was here before my changes. I suspect @michal might know more |
| 735 ↗ | (On Diff #28255) | Makes sense - this will keep the types in sync with validators too |
| lib/types/media-types.js | ||
|---|---|---|
| 688 ↗ | (On Diff #28328) | One advantage of wrapping it in tShape<T>({...}) is that flow will throw an error if the field names don't match between the validator and the type. |
| 697 ↗ | (On Diff #28255) | When I was creating the validators, I exported them if the type they were based on was also exported. |
| lib/types/media-types.js | ||
|---|---|---|
| 688 ↗ | (On Diff #28328) | Won't that happen on line 697 and 698 still? |
| lib/types/media-types.js | ||
|---|---|---|
| 688 ↗ | (On Diff #28328) | Unfortunately, the flow types for tcomb are not that smart. The field check only works if you explicitly specify the generic T in tShape<T>. |