Overview

The current implementation for building and storing the CSS compiled with Tailwind follows a naïve approach where we build the CSS for every component individually, and store it with the component.

Tailwind CSS in XB currently

There are two problems with this:

  1. CSS code is duplicated. We planned to fix this by de-duplicating, e.g. as part of the CSS aggregation.
  2. When global CSS, which is our Tailwind config, changes, the CSS compiled for our individual components becomes stale. This was a gap in our plan.

Proposed resolution

I worked on the following proposal with @effulgentsia.

  1. Store only the component-level transpiled CSS in the individual components.
  2. Maintain a list of Tailwind class name candidates in the source_js property of the xb_asset_library entity that is used for global CSS.
    1. Evaluate best format to do so. E.g. single list of strings as a JS comment vs. a structured set of class names coupled with components.
    2. Depending on 2.1., implement garbage collection logic. E.g. in case of a single list of strings, we can re-calculate what class names are unused at cron runs.
  3. Compile CSS on component markup and global CSS changes, store it in the compiled_css property of the Global CSS xb_asset_library entity.

Proposed changes for XB compiling Tailwind

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

balintbrews created an issue. See original summary.

balintbrews’s picture

Issue tags: +sprint
effulgentsia’s picture

Issue tags: -sprint

This is still an important issue to get done, but we started a shorter-than-usual sprint following DrupalCon, and are choosing to prioritize some other issues ahead of this one. We'll pick it up again in an upcoming sprint.

wim leers’s picture

Component: Page builder » Theme builder
wim leers’s picture

balintbrews’s picture

Changes by this issue are going to significantly effect the code for saving JS component data. I would like to take this opportunity and improve the auto-save logic. Here is my high-level plan.

Make auto-save more robust

Auto-save currently works by subscribing to any changes in the code editor Redux slice. The complexity of this is that there are changes that will occur upon loading the data for the first time, and we need to ensure that those are not triggering an auto-save.

The way this is currently implemented is fragile: The implementation makes the assumption that changes that occur first are from data loading, and whatever changes come after are good to auto-save. Certain race conditions can easily break this — and they have in test environments.

I would like to update that logic in a way where the actions dispatched to update the code editor can express whether auto-saving is desired. So instead of the subscriber logic trying to figure out when auto-save is appropriate, we let the individual actions tell when it is not. By default we should assume auto-save is desired, as that is the more common case.

Fix auto-save UX issues

The current auto-save implementation has a big problem. Data can be lost if users navigate away from the code editor route before saving the data has been completed. This can sometimes take up to seconds, especially in slower environments. #3513221: Auto-save changes from code editor get lost if you navigate out too quickly tracks this problem.

To mitigate this, we can delay navigation using the useBlocker hook of React Router in the following cases:

  1. Auto-save is pending;
  2. JS/CSS compilation is in progress.

Whenever users try to navigate away, we should

  1. display a toaster informing the user that saving/compilation is happening;
  2. disable code editor input;
  3. disable editing props and slots.

Once the JS/CSS compilation is finished, and auto-save is done, we'll automatically proceed with the navigation.

An important change that is required for this, which will also help making auto-save more robust in general, is that auto-save should not be subscribed to changes in the compiled JS or CSS. Instead, finishing the compilation should dispatch the auto-save action. This way we ensure that an auto-save always explicitly follows a compilation. Navigating away can be sufficiently delayed, and compiling the preview for the first time will not trigger an auto-save. (Preview is always compiled, never read from the saved data.)

balintbrews’s picture

balintbrews’s picture

Thanks, @larowlan, for the sanity check on my tests, and pointing out inefficiencies with the new CI job.

balintbrews’s picture

Assigned: balintbrews » Unassigned
Status: Active » Needs review

This is ready for a first round of review. I haven't done the "Fix auto-save UX issues" part from #7, but I added an isCompiling and isSaving status to the Redux slice. I'll move the rest of that work back to #3513221: Auto-save changes from code editor get lost if you navigate out too quickly, this MR is already enormous.

I recommend starting the review from useCodeEditor.ts and move towards the hooks used in there. To understand what's happening, there is a flowchart in the MR. I refactored codeEditorSlice.ts, which produced changes in many files: I added comments to all of those with a 🟢 emoji to help skipping over them.

The MR also introduces Vitest for component/unit tests. I really wanted to have these lower level tests for the updated functionality, and Cypress components tests are not suitable for mocking/stubbing ES modules.

jessebaker made their first commit to this issue’s fork.

balintbrews’s picture

  • balintbrews committed ce566348 on 0.x
    Issue #3516390 by balintbrews, jessebaker, effulgentsia, wim leers,...
balintbrews’s picture

Status: Needs review » Fixed
wim leers’s picture

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.