From ee3d7edefb6368bea3be28ae50675c1d5493866d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bertalan=20K=C3=B6rmendy?= Date: Mon, 21 Oct 2024 14:07:19 +0200 Subject: [PATCH] Remove unnecessary Tailwind class compilation calls (#6534) # [Editions store](https://utopia.fish/p/2e4310b0-nostalgic-blackberry/?branch_name=fix-tailwind-fine-grained-dom-observer) ## Problem The Tailwind class generation code was running twice at the end of an interaction. Instead, we only want to re-run the tailwind compilation code when - a new element is added to the DOM or the `class` attribute of an element is updated - the Tailwind config file is updated (so tweaks to the tailwind config show up right away on the canvas) ## Fix - remove the `RequireFn` param from `useTailwindCompilation`. This param is problematic because a new `requireFn` is passed to `useTailwindCompilation` whenever `projectContents` changes, which triggers a re-compilation. Instead, the require fn is constructed in a `useRefEditorState` and used through that ref - The Tailwind config file is selected with a hook using `createSelector` for memoization - The mutation observer that runs the Tailwind class generation is only run if there were new nodes added (with potentially new Tailwind classes added to the DOM) or a `class` attribute was updated (with a potentially new Tailwind class) ### Out of scope This PR doesn't deal with the problem of generating new Tailwind classes only for the elements that have changed. This is due to how the library we use for tailwind class compilation works, finding a more fine-grained way to do this would be a more involved task ### Details - The PR also adds a test that tests tailwind style generation with Remix navigation --- .../src/components/canvas/ui-jsx-canvas.tsx | 2 +- .../src/core/tailwind/tailwind-compilation.ts | 82 ++++++--- .../core/tailwind/tailwind.spec.browser2.tsx | 170 ++++++++++++++++++ 3 files changed, 230 insertions(+), 24 deletions(-) diff --git a/editor/src/components/canvas/ui-jsx-canvas.tsx b/editor/src/components/canvas/ui-jsx-canvas.tsx index 85239c3dce47..3f21fb976144 100644 --- a/editor/src/components/canvas/ui-jsx-canvas.tsx +++ b/editor/src/components/canvas/ui-jsx-canvas.tsx @@ -498,7 +498,7 @@ export const UiJsxCanvas = React.memo((props) const executionScope = scope - useTailwindCompilation(customRequire) + useTailwindCompilation() const topLevelElementsMap = useKeepReferenceEqualityIfPossible(new Map(topLevelJsxComponents)) diff --git a/editor/src/core/tailwind/tailwind-compilation.ts b/editor/src/core/tailwind/tailwind-compilation.ts index 972fee42cc9c..b71edcc1b587 100644 --- a/editor/src/core/tailwind/tailwind-compilation.ts +++ b/editor/src/core/tailwind/tailwind-compilation.ts @@ -1,9 +1,8 @@ import React from 'react' import type { TailwindConfig, Tailwindcss } from '@mhsdesign/jit-browser-tailwindcss' import { createTailwindcss } from '@mhsdesign/jit-browser-tailwindcss' -import type { ProjectContentTreeRoot, TextFile, TextFileContents } from 'utopia-shared/src/types' +import type { ProjectContentTreeRoot } from 'utopia-shared/src/types' import { getProjectFileByFilePath, walkContentsTree } from '../../components/assets' -import { interactionSessionIsActive } from '../../components/canvas/canvas-strategies/interaction-state' import { CanvasContainerID } from '../../components/canvas/canvas-types' import { Substores, @@ -18,6 +17,8 @@ import { ElementsToRerenderGLOBAL } from '../../components/canvas/ui-jsx-canvas' import { isFeatureEnabled } from '../../utils/feature-switches' import type { Config } from 'tailwindcss/types/config' import type { EditorState } from '../../components/editor/store/editor-state' +import { createSelector } from 'reselect' +import type { ProjectContentSubstate } from '../../components/editor/store/store-hook-substore-types' const LatestConfig: { current: { code: string; config: Config } | null } = { current: null } export function getTailwindConfigCached(editorState: EditorState): Config | null { @@ -87,45 +88,80 @@ function generateTailwindClasses(projectContents: ProjectContentTreeRoot, requir void generateTailwindStyles(tailwindCss, allCSSFiles) } -export const useTailwindCompilation = (requireFn: RequireFn) => { - const projectContents = useEditorState( - Substores.projectContents, - (store) => store.editor.projectContents, - 'useTailwindCompilation projectContents', +function runTailwindClassGenerationOnDOMMutation( + mutations: MutationRecord[], + projectContents: ProjectContentTreeRoot, + isInteractionActive: boolean, + requireFn: RequireFn, +) { + const updateHasNewTailwindData = mutations.some( + (m) => + m.addedNodes.length > 0 || // new DOM element was added with potentially new classes + m.attributeName === 'class', // potentially new classes were added to the class attribute of an element + ) + if ( + !updateHasNewTailwindData || + isInteractionActive || + ElementsToRerenderGLOBAL.current !== 'rerender-all-elements' // implies that an interaction is in progress) + ) { + return + } + generateTailwindClasses(projectContents, requireFn) +} + +const tailwindConfigSelector = createSelector( + (store: ProjectContentSubstate) => store.editor.projectContents, + (projectContents) => getProjectFileByFilePath(projectContents, TailwindConfigPath), +) + +export const useTailwindCompilation = () => { + const requireFnRef = useRefEditorState((store) => { + const requireFn = store.editor.codeResultCache.curriedRequireFn(store.editor.projectContents) + return (importOrigin: string, toImport: string) => requireFn(importOrigin, toImport, false) + }) + const projectContentsRef = useRefEditorState((store) => store.editor.projectContents) + + const isInteractionActiveRef = useRefEditorState( + (store) => store.editor.canvas.interactionSession != null, ) - const isInteractionActiveRef = useRefEditorState((store) => - interactionSessionIsActive(store.editor.canvas.interactionSession), + // this is not a ref, beacuse we want to re-compile the Tailwind classes when the tailwind config changes + const tailwindConfig = useEditorState( + Substores.projectContents, + tailwindConfigSelector, + 'useTailwindCompilation tailwindConfig', ) - const observerCallback = React.useCallback(() => { + React.useEffect(() => { + const canvasContainer = document.getElementById(CanvasContainerID) if ( - isInteractionActiveRef.current || - ElementsToRerenderGLOBAL.current !== 'rerender-all-elements' || // implies that an interaction is in progress + tailwindConfig == null || // TODO: read this from the utopia key in package.json + canvasContainer == null || !isFeatureEnabled('Tailwind') ) { return } - generateTailwindClasses(projectContents, requireFn) - }, [isInteractionActiveRef, projectContents, requireFn]) - React.useEffect(() => { - const tailwindConfigFile = getProjectFileByFilePath(projectContents, TailwindConfigPath) - if (tailwindConfigFile == null || tailwindConfigFile.type !== 'TEXT_FILE') { - return // we consider tailwind to be enabled if there's a tailwind config file in the project - } - const observer = new MutationObserver(observerCallback) + const observer = new MutationObserver((mutations) => { + runTailwindClassGenerationOnDOMMutation( + mutations, + projectContentsRef.current, + isInteractionActiveRef.current, + requireFnRef.current, + ) + }) - observer.observe(document.getElementById(CanvasContainerID)!, { + observer.observe(canvasContainer, { attributes: true, childList: true, subtree: true, }) - observerCallback() + // run the initial tailwind class generation + generateTailwindClasses(projectContentsRef.current, requireFnRef.current) return () => { observer.disconnect() } - }, [isInteractionActiveRef, observerCallback, projectContents, requireFn]) + }, [isInteractionActiveRef, projectContentsRef, requireFnRef, tailwindConfig]) } diff --git a/editor/src/core/tailwind/tailwind.spec.browser2.tsx b/editor/src/core/tailwind/tailwind.spec.browser2.tsx index b95956046c32..91b3eb6447a0 100644 --- a/editor/src/core/tailwind/tailwind.spec.browser2.tsx +++ b/editor/src/core/tailwind/tailwind.spec.browser2.tsx @@ -1,5 +1,13 @@ +import { mouseClickAtPoint } from '../../components/canvas/event-helpers.test-utils' +import type { EditorRenderResult } from '../../components/canvas/ui-jsx.test-utils' import { renderTestEditorWithModel } from '../../components/canvas/ui-jsx.test-utils' +import { switchEditorMode } from '../../components/editor/actions/action-creators' +import { EditorModes } from '../../components/editor/editor-modes' +import { StoryboardFilePath } from '../../components/editor/store/editor-state' +import { createModifiedProject } from '../../sample-projects/sample-project-utils.test-utils' import { setFeatureForBrowserTestsUseInDescribeBlockOnly } from '../../utils/utils.test-utils' +import { windowPoint } from '../shared/math-utils' +import { TailwindConfigPath } from './tailwind-config' import { Project } from './tailwind.test-utils' describe('rendering tailwind projects in the editor', () => { @@ -133,4 +141,166 @@ describe('rendering tailwind projects in the editor', () => { }) } }) + + describe('Remix', () => { + const projectWithMultipleRoutes = createModifiedProject({ + [StoryboardFilePath]: `import * as React from 'react' + import { RemixScene, Storyboard } from 'utopia-api' + + export var storyboard = ( + + + + ) + `, + ['/app/root.js']: `import React from 'react' + import { Outlet } from '@remix-run/react' + + export default function Root() { + return ( +
+ I am Root! + +
+ ) + } + `, + ['/app/routes/_index.js']: `import React from 'react' + import { Link } from '@remix-run/react' + + export default function Index() { + return ( +
+ Index page + About +
+ ) + } + `, + ['/app/routes/about.js']: `import React from 'react' + + export default function About() { + return ( +
+ About page +
+ ) + } + `, + '/src/app.css': ` + @tailwind base; + @tailwind components; + @tailwind utilities; + `, + [TailwindConfigPath]: ` + const Tailwind = { + theme: { + colors: { + transparent: 'transparent', + current: 'currentColor', + white: '#ffffff', + purple: '#3f3cbb', + midnight: '#121063', + metal: '#565584', + tahiti: '#3ab7bf', + silver: '#ecebff', + 'bubble-gum': '#ff77e9', + bermuda: '#78dcca', + }, + }, + plugins: [ + function ({ addUtilities }) { + const newUtilities = { + '.text-shadow': { + textShadow: '2px 2px 4px rgba(0, 0, 0, 0.1)', + }, + '.text-shadow-md': { + textShadow: '3px 3px 6px rgba(0, 0, 0, 0.2)', + }, + '.text-shadow-lg': { + textShadow: '4px 4px 8px rgba(0, 0, 0, 0.3)', + }, + '.text-shadow-none': { + textShadow: 'none', + }, + } + + addUtilities(newUtilities, ['responsive', 'hover']) + }, + ], + } + export default Tailwind`, + }) + + it('can render content in a RemixScene', async () => { + const editor = await renderTestEditorWithModel( + projectWithMultipleRoutes, + 'await-first-dom-report', + ) + { + const root = editor.renderedDOM.getByTestId('root') + const { backgroundColor, display, flexDirection, gap, fontSize } = getComputedStyle(root) + expect({ backgroundColor, display, flexDirection, gap, fontSize }).toEqual({ + backgroundColor: 'rgba(0, 0, 0, 0)', + display: 'flex', + flexDirection: 'column', + fontSize: '24px', + gap: '40px', + }) + } + { + const index = editor.renderedDOM.getByTestId('index') + const { display, flexDirection, gap } = getComputedStyle(index) + expect({ display, flexDirection, gap }).toEqual({ + display: 'flex', + flexDirection: 'column', + gap: '32px', + }) + } + }) + it('can render content after navigating to a different page', async () => { + const editor = await renderTestEditorWithModel( + projectWithMultipleRoutes, + 'await-first-dom-report', + ) + await switchToLiveMode(editor) + await clickRemixLink(editor) + + { + const about = editor.renderedDOM.getByTestId('about') + const { display, flexDirection, gap, padding } = getComputedStyle(about) + expect({ display, flexDirection, gap, padding }).toEqual({ + display: 'flex', + flexDirection: 'row', + gap: '24px', + padding: '16px', + }) + } + { + const aboutText = editor.renderedDOM.getByTestId('about-text') + const { textShadow } = getComputedStyle(aboutText) + expect(textShadow).toEqual('rgba(0, 0, 0, 0.2) 3px 3px 6px') + } + }) + }) }) + +const switchToLiveMode = (editor: EditorRenderResult) => + editor.dispatch([switchEditorMode(EditorModes.liveMode())], true) + +async function clickLinkWithTestId(editor: EditorRenderResult, testId: string) { + const targetElement = editor.renderedDOM.queryAllByTestId(testId)[0] + const targetElementBounds = targetElement.getBoundingClientRect() + + const clickPoint = windowPoint({ x: targetElementBounds.x + 5, y: targetElementBounds.y + 5 }) + await mouseClickAtPoint(targetElement, clickPoint) +} + +async function clickRemixLink(editor: EditorRenderResult) { + await clickLinkWithTestId(editor, 'remix-link') + await editor.getDispatchFollowUpActionsFinished() +}