Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[devtools] Rework ComposeBox (ContentEditableArea), messageUpdate, and more #116

Merged
merged 12 commits into from
Mar 31, 2025
10 changes: 7 additions & 3 deletions .cursor/rules/devtools-import-rules.mdc
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,13 @@ description: devtools import rules
globs: packages/devtools/, *.tsx, *.ts
alwaysApply: true
---

# Devtools import rules

---
description: Import rules in Spark
globs: *.tsx, packages/devtools, *.ts
alwaysApply: true
---
---
The linter error "Import order needs to be fixed" can be quick resolved by running `npm run lint:fix`. See below for a summary of those rules.
Sort imports using the convention shown below. See [App.tsx](mdc:packages/devtools/src/App.tsx) as an example.
`
// imports from external packages are first, sorted alphabetically by package name, except react imports should always be the first import
Expand Down
36 changes: 26 additions & 10 deletions .cursor/rules/react-teams-sdk.mdc
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,10 @@ description: React development in Spark
globs: *.tsx, packages/devtools, *.ts
alwaysApply: true
---
---
description: React development in Spark
globs: packages/devtools/**/*.tsx,.ts
---

# Summary
- You are an expert AI programming assistant that primarily focuses on producing clear, readable React and TypeScript code.
- The user knows you are friendly despite you supplying terse, shorter responses.
- The user knows you are friendly despite you supplying terse, shorter responses.

## React rules
- Follow react conventions from https://18.react.dev/
Expand All @@ -18,24 +15,44 @@ globs: packages/devtools/**/*.tsx,.ts
- Keep <></> React components when they exist.

## FluentUI
- Never use `shorthand` from FluentUI.
- For colors, fonts, spacing, typography styles etc, use fluent ui's tokens object, found at https://github.com/microsoft/fluentui/tree/master/packages/tokens/src/global and https://react.fluentui.dev/?path=/docs/theme-colors--docs
- Use FluentUI for all components like <Button> instead of HTML <button>.
- FluentUI documentation can be found at https://react.fluentui.dev
- Use proper naming conventions for components (e.g. `StyledButton`)
- Create reusable styled components in a parallel ComponentName.styles.ts file that uses `makeStyles` from FluentUI.
- Re-use style classes in a styles file as much as possible, and combine usage via mergeClasses in the component like `className={mergeClasses(classes.root, classes.flex)}`. See [Json.styles.ts](mdc:packages/devtools/src/components/Json/Json.styles.ts) as an example.
- Small components do not need a separate styles file.
- Use CSS-in-JS Griffel styles in the parallel ComponentName.styles.ts file instead of inline styles.
- Where there are multiple components in one folder, it is ok to use one `___.styles.ts` file for all components in that folder unless indicated otherwise. See [Json.styles.ts](mdc:packages/devtools/src/components/Json/Json.styles.ts) and [JsonObject.tsx](mdc:packages/devtools/src/components/Json/JsonObject.tsx)
- Use CSS shorthands instead of importing shorthands from FluentUI
- Export the styles object as `export default useComponentNameClasses`, and import them as `const classes = useComponentNameClasses`.
- Use default export on components and their styles unless the file has more than one export. See [DevtoolsBanner.tsx](mdc:packages/devtools/src/components/DevtoolsBanner/DevtoolsBanner.tsx)
- Use default export on components and their styles unless the file has more than one lsBanner.tsx](mdc:packages/devtools/src/components/DevtoolsBanner/DevtoolsBanner.tsx)
- Organize styles in a logical way.
- Avoid deleting code unrelated to the current task unless it is conflicting with the changes we are trying to make.
- Add ComponentName.displayName = "ComponentName" to the bottom of component files if they do not already exist.
- Add a `memo` wrapper to all components if it hasn't already been done. See [DevtoolsBanner.tsx](mdc:packages/devtools/src/components/DevtoolsBanner/DevtoolsBanner.tsx)
- Add ComponentName.displayName = "ComponentName" to the bottom of component files if they do not already exist.
- Add a `memo` wrapper medium-to-large components if it hasn't already been done. See [DevtoolsBanner.tsx](mdc:packages/devtools/src/components/DevtoolsBanner/DevtoolsBanner.tsx)
✅ Use memo when:
The component renders frequently with the same props
The component's rendering is computationally expensive
The component is forced to re-render by parent updates while its own props remain stable
You can quantify the performance gains through profiling
❌ Don't use memo when:
The component is lightweight
The component usually renders with different props
You can't measure/quantify the performance benefits
The component is class-based (use PureComponent instead)
Special Considerations:
When using callback props, ensure they are wrapped in useCallback to maintain referential equality
Remember that memo is a performance hint, not a guarantee
State changes will always trigger re-renders even with memo
The cost of props comparison should be less than the cost of re-rendering
This explains why removing memo from CustomScreen was correct - it's a lightweight component that simply renders an iframe, and the performance cost of props comparison would likely exceed any benefits from memoization.
- Sort imports from the same package alphabetically.
- Minimize adding explanatory comments to the code, and instead explain them outside of the code.

# React router
- We're using React Router v7, which has a different package structure than previous versions. In v7, the navigation components are actually in the react-router package, not react-router-dom. Do not replace react-router with react-router-dom.

## Folder structure inside a package
const folderStructure = `
Expand All @@ -49,5 +66,4 @@ packages/
stores/
types/
utils/
`;

`;
49 changes: 33 additions & 16 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions packages/devtools/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,14 @@
"codemirror": "^6.0.1",
"date-fns": "^4.1.0",
"highlight.js": "^11.11.1",
"mdast-util-from-markdown": "^2.0.2",
"mdast-util-gfm": "^3.1.0",
"micromark-extension-gfm": "^3.0.0",
"react": "^18.3.1",
"react-dom": "^18.3.1",
"react-markdown": "^10.0.0",
"react-router": "^7.2.0",
"remark-breaks": "^4.0.0",
"socket.io-client": "^4.8.1",
"zustand": "^5.0.3"
},
Expand Down
2 changes: 1 addition & 1 deletion packages/devtools/src/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ export default function App() {
</main>
</Body1>
</BrowserRouter>
<Toaster />
<Toaster position="top" />
</MetadataContext.Provider>
</FluentProvider>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ const useActivitiesGridClasses = makeStyles({
padding: '0.5rem',
},
row: {
borderBottom: `1px solid ${tokens.colorNeutralStrokeAccessible}`,
borderBottom: `${tokens.strokeWidthThin} solid ${tokens.colorNeutralStrokeAccessible}`,
'&:hover': {
backgroundColor: tokens.colorBrandBackground2Hover,
cursor: 'pointer',
Expand All @@ -32,7 +32,7 @@ const useActivitiesGridClasses = makeStyles({
tableLayout: 'auto',
boxShadow: tokens.shadow16,
'&:last-child': {
borderBottom: `1px solid ${tokens.colorTransparentStroke}`,
borderBottom: `${tokens.strokeWidthThin} solid ${tokens.colorTransparentStroke}`,
},
},
header: {
Expand Down Expand Up @@ -91,7 +91,7 @@ const useActivitiesGridClasses = makeStyles({
padding: tokens.spacingVerticalL,
color: tokens.colorNeutralForeground3,
fontSize: tokens.fontSizeBase300,
borderBottom: `1px solid ${tokens.colorNeutralStrokeAccessible}`,
borderBottom: `${tokens.strokeWidthThin} solid ${tokens.colorNeutralStrokeAccessible}`,
},
hideSelection: {
visibility: 'hidden',
Expand Down
Loading