[chat] Enforce top-level type imports - #23037
Conversation
Add the chat packages to the consistent-type-imports / import-consistent-type-specifier-style block (top-level `import type {}`), following the existing enforcement for charts, data-grid, date-pickers, scheduler, and tree-view.
Also manually converts one `import()` type annotation in chat-stream.ts to a top-level `import type`, which the rule forbids but cannot autofix.
Mechanical eslint --fix + prettier result for the rule enabled in the previous commit.
Deploy previewBundle size
Check out the code infra dashboard for more information about this PR. |
| export interface ChatStartMessageChunk { | ||
| type: 'start'; | ||
| messageId: string; | ||
| author?: import('./chat-entities').ChatUser; |
There was a problem hiding this comment.
@hasdfa was this intentionally a dynamic import?
There was a problem hiding this comment.
Ah, no, it feels like just it should not be dynamic import and I do not remember that there was some special case, so it's okay to replace it with normal import. Thanks!
There was a problem hiding this comment.
Some of the file names are kebab case, and some seem to be pascal/camelcase. Is there a rationale to having a few files kebab case, or is this unintential? The repo seems to heavily favor the latter
┌───────────────────────────────┬─────────────┬──────────────┬───────┐
│ Package │ kebab │ Pascal/camel │ total │
├───────────────────────────────┼─────────────┼──────────────┼───────┤
│ x-telemetry │ 12 (100.0%) │ 0 (0.0%) │ 12 │
├───────────────────────────────┼─────────────┼──────────────┼───────┤
│ x-charts-vendor │ 2 (100.0%) │ 0 (0.0%) │ 2 │
├───────────────────────────────┼─────────────┼──────────────┼───────┤
│ x-codemod │ 195 (95.1%) │ 10 (4.9%) │ 205 │
├───────────────────────────────┼─────────────┼──────────────┼───────┤
│ x-data-grid-generator │ 6 (18.2%) │ 27 (81.8%) │ 33 │
├───────────────────────────────┼─────────────┼──────────────┼───────┤
│ x-scheduler-internals-premium │ 5 (6.2%) │ 76 (93.8%) │ 81 │
├───────────────────────────────┼─────────────┼──────────────┼───────┤
│ x-scheduler-internals │ 9 (5.9%) │ 143 (94.1%) │ 152 │
├───────────────────────────────┼─────────────┼──────────────┼───────┤
│ x-chat-headless │ 9 (5.6%) │ 152 (94.4%) │ 161 │
├───────────────────────────────┼─────────────┼──────────────┼───────┤
│ x-scheduler │ 6 (3.9%) │ 148 (96.1%) │ 154 │
├───────────────────────────────┼─────────────┼──────────────┼───────┤
│ x-date-pickers-pro │ 4 (2.9%) │ 136 (97.1%) │ 140 │
├───────────────────────────────┼─────────────┼──────────────┼───────┤
│ x-date-pickers │ 5 (1.5%) │ 337 (98.5%) │ 342 │
├───────────────────────────────┼─────────────┼──────────────┼───────┤
│ x-charts-pro │ 3 (1.5%) │ 198 (98.5%) │ 201 │
├───────────────────────────────┼─────────────┼──────────────┼───────┤
│ x-charts │ 4 (0.6%) │ 624 (99.4%) │ 628 │
├───────────────────────────────┼─────────────┼──────────────┼───────┤
│ x-data-grid │ 1 (0.2%) │ 446 (99.8%) │ 447 │
├───────────────────────────────┼─────────────┼──────────────┼───────┤
│ x-virtualizer │ 0 (0.0%) │ 2 (100.0%) │ 2 │
├───────────────────────────────┼─────────────┼──────────────┼───────┤
│ x-tree-view-pro │ 0 (0.0%) │ 18 (100.0%) │ 18 │
├───────────────────────────────┼─────────────┼──────────────┼───────┤
│ x-tree-view │ 0 (0.0%) │ 74 (100.0%) │ 74 │
├───────────────────────────────┼─────────────┼──────────────┼───────┤
│ x-scheduler-premium │ 0 (0.0%) │ 43 (100.0%) │ 43 │
├───────────────────────────────┼─────────────┼──────────────┼───────┤
│ x-license │ 0 (0.0%) │ 16 (100.0%) │ 16 │
├───────────────────────────────┼─────────────┼──────────────┼───────┤
│ x-internals │ 0 (0.0%) │ 44 (100.0%) │ 44 │
├───────────────────────────────┼─────────────┼──────────────┼───────┤
│ x-internal-gestures │ 0 (0.0%) │ 104 (100.0%) │ 104 │
├───────────────────────────────┼─────────────┼──────────────┼───────┤
│ x-data-grid-pro │ 0 (0.0%) │ 124 (100.0%) │ 124 │
├───────────────────────────────┼─────────────┼──────────────┼───────┤
│ x-data-grid-premium │ 0 (0.0%) │ 149 (100.0%) │ 149 │
├───────────────────────────────┼─────────────┼──────────────┼───────┤
│ x-chat │ 0 (0.0%) │ 104 (100.0%) │ 104 │
├───────────────────────────────┼─────────────┼──────────────┼───────┤
│ x-charts-premium │ 0 (0.0%) │ 153 (100.0%) │ 153 │
└───────────────────────────────┴─────────────┴──────────────┴───────┘
Worth it to encode in lint rules?
For reference, mui/material-ui is also almost exclusively the latter
┌──────────────────────────┬─────────────┬─────────────────┬────────┐
│ Package │ kebab │ Pascal/camel │ total │
├──────────────────────────┼─────────────┼─────────────────┼────────┤
│ mui-codemod │ 444 (89.0%) │ 55 (11.0%) │ 499 │
├──────────────────────────┼─────────────┼─────────────────┼────────┤
│ mui-lab │ 1 (1.1%) │ 86 (98.9%) │ 87 │
├──────────────────────────┼─────────────┼─────────────────┼────────┤
│ mui-material │ 1 (0.1%) │ 882 (99.9%) │ 883 │
├──────────────────────────┼─────────────┼─────────────────┼────────┤
│ mui-utils │ 0 (0.0%) │ 94 (100.0%) │ 94 │
├──────────────────────────┼─────────────┼─────────────────┼────────┤
│ mui-system │ 0 (0.0%) │ 98 (100.0%) │ 98 │
├──────────────────────────┼─────────────┼─────────────────┼────────┤
│ mui-private-theming │ 0 (0.0%) │ 5 (100.0%) │ 5 │
├──────────────────────────┼─────────────┼─────────────────┼────────┤
│ mui-styled-engine │ 0 (0.0%) │ 5 (100.0%) │ 5 │
├──────────────────────────┼─────────────┼─────────────────┼────────┤
│ mui-styled-engine-sc │ 0 (0.0%) │ 4 (100.0%) │ 4 │
├──────────────────────────┼─────────────┼─────────────────┼────────┤
│ mui-styled-engine-noop │ 0 (0.0%) │ 4 (100.0%) │ 4 │
├──────────────────────────┼─────────────┼─────────────────┼────────┤
│ mui-material-pigment-css │ 0 (0.0%) │ 5 (100.0%) │ 5 │
├──────────────────────────┼─────────────┼─────────────────┼────────┤
│ mui-material-nextjs │ 0 (0.0%) │ 4 (100.0%) │ 4 │
├──────────────────────────┼─────────────┼─────────────────┼────────┤
│ mui-types │ 0 (0.0%) │ 1 (100.0%) │ 1 │
├──────────────────────────┼─────────────┼─────────────────┼────────┤
│ mui-icons-material │ 0 (0.0%) │ 10,749 (100.0%) │ 10,749 │
└──────────────────────────┴─────────────┴─────────────────┴────────┘
There was a problem hiding this comment.
Seems like newer packages more lean to kebab then camel)
Btw, it's not intentional to mix them so probably we should decide, at least per package which style do we want to go thought, or do you feel we should enforce it for whole repo?
There was a problem hiding this comment.
or do you feel we should enforce it for whole repo?
If we were to adopt a convention we should make it company wide. But I wouldn't call it top priority.
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
…ports # Conflicts: # packages/x-chat/src/ChatBox/ChatBox.tsx # packages/x-chat/src/ChatConversationList/ChatConversationList.tsx # packages/x-chat/src/ChatIndicators/ChatStreamingIndicatorRow.tsx # packages/x-chat/src/ChatMessage/ChatMessage.tsx
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
|
@claude review low |
This comment was marked as resolved.
This comment was marked as resolved.
…ports # Conflicts: # packages/x-chat/src/ChatBox/ChatBox.types.ts # packages/x-chat/src/ChatCodeBlock/ChatCodeBlock.tsx # packages/x-chat/src/ChatComposer/ChatComposerAttachmentList.tsx # packages/x-chat/src/ChatMessage/ChatMessageInlineMeta.tsx # packages/x-chat/src/ChatMessage/ChatMessageMeta.tsx
Adds the chat packages to the top-level type-imports rule (
consistent-type-importswithseparate-type-imports+import/consistent-type-specifier-style: prefer-top-level), following the same enforcement already in place for charts, data-grid, date-pickers (#22679), scheduler (#22931), and tree-view (#23034).Commits:
import()type annotation inchat-stream.ts(the rule forbids these but cannot autofix them).eslint --fix+prettierresult.