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

Remove all global css pollution #6032

Open
1 of 2 tasks
Airkro opened this issue Nov 30, 2021 · 29 comments · Fixed by gpbl/react-day-picker#1817
Open
1 of 2 tasks

Remove all global css pollution #6032

Airkro opened this issue Nov 30, 2021 · 29 comments · Fixed by gpbl/react-day-picker#1817
Labels
external This issue is caused by an external dependency and not Docusaurus. proposal This issue is a proposal, usually non-trivial change

Comments

@Airkro
Copy link

Airkro commented Nov 30, 2021

Have you read the Contributing Guidelines on issues?

Motivation

Global css messes up many things, please consider removing them (rewrite in classes), it will save the day.

image

Self-service

  • I'd be willing to do some initial work on this proposal myself.
@Airkro Airkro added status: needs triage This issue has not been triaged by maintainers proposal This issue is a proposal, usually non-trivial change labels Nov 30, 2021
@Josh-Cena Josh-Cena removed the status: needs triage This issue has not been triaged by maintainers label Nov 30, 2021
@Josh-Cena
Copy link
Collaborator

Agree that this is a valid concern that I have been asked on Discord. However, several things missing from the proposal that I wish we can get response on:

  1. Impact. Is there a really strong use-case that's hindered by the global CSS? Won't all: revert work if you want a part of the page unstyled?
  2. Possible implementation. How would we work to implement your proposal? Many of these elements are directly generated from Markdown (e.g. <kbd>), which is also the primary focus of Docusaurus. Does that mean we should map all Markdown elements to one with a class name?

Global css messes up many things

This is an assertion and I don't see exact what it messes up

please consider removing them (rewrite in classes)

You should report that on the Infima side, the CSS framework developed alongside Docusaurus. In Docusaurus we are more concerned about how CSS classes can interoperate with the current way that HTML is generated.

@Josh-Cena Josh-Cena added the status: needs more information There is not enough information to take action on the issue. label Nov 30, 2021
@Josh-Cena Josh-Cena changed the title Remove all global css polution Remove all global css pollution Nov 30, 2021
@slorber
Copy link
Collaborator

slorber commented Nov 30, 2021

IMHO we could scope those rules under a .infima-app class or something used at the root of the Docusaurus app.

But would this solve your problem @Airkro ?

Please describe your actual problem. Include screenshots if possible to show broken styling.

@Josh-Cena
Copy link
Collaborator

we could scope those rules under a .infima-app class or something used at the root of the Docusaurus app.

The OP wants to easily opt-out from Infima. Scoping probably doesn't prevent the tag selectors from being applied to every element within the <Layout>

@tobemaster56
Copy link

tobemaster56 commented Jan 21, 2022

I meet this problem too

We are building our own React component library. There are a lot of documents based on docusaurus. Thanks to the mdx of docusaurus, we can easily render React components in the document. We need to create a lot of demos. We have a pagination component, the li element is used, but in the demo, the style is a bit abnormal. I found out that it was a problem with the global styles of docusaurus.

I created a discussion css style isolation about it, with detailed instructions and screenshots, and I tried all:revert, but it didn't work.

While we can override the relevant styles to solve this kind of problem temporarily, I really feel that this is not the best solution and is a bit ugly. Soon after I discovered the problem, another colleague had the same problem, a calendar demo in which the style of the tables in the component was contaminated, and it took him a lot of time to find out the cause.

I hope there is a once and for all way to be free from global styles.

After discussing it with @Josh-Cena , I tried to use [react-shadow-root]https://www.npmjs.com/package/react, but in the end, it failed, and for our component library, our styles were also global and blocked by shadow-dom. I gave up

In the end, I still hope that the official will come up with an elegant plan, and I hope to hear a response as soon as possible.

@slorber
Copy link
Collaborator

slorber commented Jan 21, 2022

Agree, it should be possible to mount a component library in Docusaurus and expect it to not be affected by Docusaurus styles (without requiring the usage of an iframe)

We'll look into this.

In the meantime please report here any Docusaurus style that is affecting the styling of your components. The more exhaustive the list, the better.

@Josh-Cena
Copy link
Collaborator

What about having a special class in Infima that reverts all Infima styles? I still find value in certain global styles instead of moving everything to class names.

@Airkro
Copy link
Author

Airkro commented Jan 22, 2022

Sorry for not responding. I integrated Swagger UI with Docusaurus. I have to render Swagger UI in shadow dom to avoid CSS pollution.

@Josh-Cena
Copy link
Collaborator

Josh-Cena commented Mar 25, 2022

Closing this since it's unactionable on Docusaurus side. We can discuss in Infima instead, see also facebookincubator/infima#8

I think the easiest solution is to provide an OOTB Sandbox component which is basically a shadow DOM? A reset utility class can do the job as well.

@Josh-Cena Josh-Cena added external This issue is caused by an external dependency and not Docusaurus. and removed status: needs more information There is not enough information to take action on the issue. labels Mar 25, 2022
@slorber
Copy link
Collaborator

slorber commented Apr 20, 2022

I'll re-open because we can also do something on Docusaurus side, such as providing another theme (likely Tailwind) that does not have this problem in the first place.

It should be possible to mount MDX components in your doc and have those components not affected by Docusaurus theme unscoped styles.

Now we can also open another discussion on Infima to ensure all styles there are scoped properly.

@slorber slorber reopened this Apr 20, 2022
@Josh-Cena
Copy link
Collaborator

Josh-Cena commented Apr 20, 2022

I still don't understand this. First is "why", second is "how".

Infima is more comparable to Bootstrap than Tailwind. We don't ask users to compose class names to get any useful styles; we directly target elements. If user writes <kbd>foo</kbd> she should immediately get styling that looks consistent with the rest of the design, instead of having to compose layers of class names to adjust the margin, the border, the color... Its goal of driving content-centric website effectively means users should stop caring about styling in all and delegate everything to the default styles.

Let's say the use-case of showcasing custom components is justified. How do we scope Infima? We inevitably have to do it "the Tailwind way", i.e. use class names instead of tag selectors everywhere. But does that sound like the best solution?

  • It instantly means we have to do A LOT more MDX component mapping because literally everything, from ul to table to hr, has Infima-default styles today. We have to transform them to something like <table className="table" />.
  • Furthermore, it means React pages also have to have extra class names just to get the same styling as Markdown pages. I already have a hard time explaining to people that MDX != HTML != JSX because there can be build-time transformations. Simply transforming asset links has already confused users when [link](./asset.pdf) works in Markdown but <a href="./asset.pdf">link</a> doesn't work in JSX. (Which led to my doc addition that Markdown is declarative). Now we not only want to take the intuition of Markdown -> JSX mapping away, but also the JSX -> JSX mapping intuition away. Being intuitive—not being perfect for every use-case—is what makes us competitive.

Now, can this be done WITHOUT "remove all global css polution"? Yes. We can do it in userland today with iframes and/or shadow DOM and/or a reset class. Those all sound like viable solutions that (a) is extensible (b) is robust (c) doesn't require any work from Docusaurus.

Taking a step further back, how can Docusaurus help with this use-case? This should be discussed in Infima instead of Docusaurus. Until we decide which path we want to take within an Infima thread, there doesn't seem to be anything Docusaurus can do, if we need to do anything at all.

@slorber
Copy link
Collaborator

slorber commented Apr 20, 2022

It instantly means we have to do A LOT more MDX component mapping because literally everything, from ul to table to hr, has Infima-default styles today. We have to transform them to something like <table className="table" />.

We'll likely have to do all that anyway if we want to support a tailwind theme, unless you also want Tailwind to apply global classes?

We can do it in userland today with iframes and/or shadow DOM and/or a reset class.

  • iframes is not really a good solution. People want to embed native runnable code in their doc.
  • shadow DOM: looks like an interesting thing
  • reset: why not but then do we want users to have to maintain this class themselves over time?

I'm open to exploring the shadow DOM solution.

Maybe the issue is not global CSS, but simply the inability to embed natively your lib's code in your doc without having any styling conflicts?

If we have a solution to this problem, with documentation and some dogfooding, maybe we don't have to handle the global unscoped CSS issue

@Josh-Cena
Copy link
Collaborator

Josh-Cena commented Apr 20, 2022

Yeah, if we decide to go with shadow DOM it could be most relevant to Docusaurus (we can even offer a theme component like Sandbox). I think someone did tell me they solved it this way.

unless you also want Tailwind to apply global classes?

That I'm actually not sure. Tailwind is, really, a reusable styles library rather than a reusable components library (which Bootstrap/MUI/Infima are more of). To some degree, that contradicts with the goal of a content-centric rather than design-centric site. Maybe I'm okay with the Tailwind theme doing more MDX component mapping? Certainly don't want it to be a widespread pattern though.

@Josh-Cena
Copy link
Collaborator

Josh-Cena commented Apr 20, 2022

Also note that if a user opts into Tailwind I'd assume she's already aware of the Tailwind ideology (which may not be at all apparent to other casual developers), so the JSX/MDX behavior deviation may be less important. But for the classic theme, which I still aim to be the most accessible for everyone in the long term, should be intuitive enough.

@gpbl
Copy link

gpbl commented Apr 21, 2022

Thanks for reopening this!

Please note this is not just related to the markdown rendering. Even outside the markdown docs, there are selectors polluting the global scope, which affects also regular pages/routes. So I'm not sure if the "shadow" solution would work.

For example, this is a route in Docusaurus: https://react-day-picker.js.org/render?example=start. The style is not correct because the CSS overrides the table head style:

table th {
  background-color: var(--ifm-table-head-background);
  color: var(--ifm-table-head-color);
  font-weight: var(--ifm-table-head-font-weight);
}

We shouldn't see these selectors in the CSS. However, reverting them would affect the existing implementations, so fixing this is impracticable.

Maybe it's time to develop a new theme? I evaluated https://docusaurus-theme-no-style.netlify.app, to start implementing my own, but that is a lot of work.

@gpbl
Copy link

gpbl commented Apr 21, 2022

Now, can this be done WITHOUT "remove all global css polution"? Yes. We can do it in userland today with iframes and/or shadow DOM and/or a reset class. Those all sound like viable solutions that (a) is extensible (b) is robust (c) doesn't require any work from Docusaurus

@Josh-Cena those are not viable solutions. Iframe is still affected by CSS pollution, and a "reset" class will never work well. To me, writing the CSS that way has been a bad choice from start, and it has nothing to do with MDX mapping or the "tailwind" way.

I understand that Infima intends to replicate Bootstrap philosophy (even if Bootstrap splits the global CSS classes in a "reboot" CSS). I question if Infima is the right choice for Docusaurus: importing a components in MDX, and having its style overridden by Docusaurus, makes the MDX feature much less useful.

@Josh-Cena
Copy link
Collaborator

Josh-Cena commented Apr 21, 2022

Infima is developed for Docusaurus, so asking if Infima is the right choice is asking if theme-classic is the right choice😅 If there are things going wrong we can fix it.

Even outside the markdown docs, there are selectors polluting the global scope,

I may have failed to make it, because that's literally my entire point. There should be minimal effort to make a non-MD page look exactly like an MD page. For example,

## Keyboard

Press <kbd>Ctrl</kbd> + <kbd>C</kbd>.

...should be directly replicable in a React file as...

function Section() {
  return (
    <>
      <h2>Keyboard</h2>
      <p>Press <kbd>Ctrl</kbd> + <kbd>C</kbd>.</p>
    </>
  );
}

...instead of forcing most users to write...

function Section() {
  return (
    <>
      <h2 className="h2">Keyboard</h2>
      <p className="p">Press <kbd className="kbd">Ctrl</kbd> + <kbd className="kbd">C</kbd>.</p>
    </>
  );
}

That's (a) tedious (b) unintuitive (c) easily inviting for inconsistent design. Through the years, we figured that it's hardly possible to please both power users and users who didn't even know React is before this, so it's better if the bar of entry can be low and things "just work", even if it becomes more headaches for power users.

@Josh-Cena
Copy link
Collaborator

Shadow DOM should be the way to go, because that's literally the purpose of shadow DOM. I didn't investigate, but I do know of successful integration cases.

@gpbl
Copy link

gpbl commented Apr 21, 2022

Infima is developed for Docusaurus, so asking if Infima is the right choice is asking if theme-classic is the right choice

Infima has tons of quirks, I've spent countless hours trying to workaround them. It is not the right theme for documenting a components library – in this case, it would be nice a note in the docs. Anyway here we are, reporting our issues with it. Hopefully soon we can have an alternative theme!

@Josh-Cena
Copy link
Collaborator

Yes, Infima is not designed with component library docs in mind—we do not expect such use-case. Using an alternative theme like Tailwind should be able to fix it.

@gpbl
Copy link

gpbl commented Apr 21, 2022

Yes, Infima is not designed with component library docs in mind—we do not expect such use-case.

I can spot few use cases in the Showcase gallery - e.g https://ionicframework.com/docs/components
I'm sure they just run into our issues, give up and/or solved it using Codesandbox.

The case of react-day-picker.js.org is not even a component library - just a single component.

@Josh-Cena
Copy link
Collaborator

Ionic solved it through shadow DOM which is what I've been saying all along:

image

Plus they are a native component library so they have a different battle.

@slorber
Copy link
Collaborator

slorber commented Jun 6, 2022

Looks like upcoming CSS @scope this could be a good solution:

CleanShot 2022-06-06 at 13 08 31@2x

https://twitter.com/sebastienlorber/status/1533767305875824641

@TimonVS
Copy link

TimonVS commented Jun 9, 2022

Our team has been using ReactShadow to workaround this issue. But we haven't been able to get it to work with Emotion yet (tried creating a separate cache and having the styles be inserted in the shadow root, but haven't managed to get it to work yet).

@stevenpetryk
Copy link

stevenpetryk commented Jul 31, 2022

Just reporting here that at Discord we've had some success using shadow DOM as well. We had to tell webpack to split the CSS bundle into three CSS files using splitChunks:

  • docusaurus-styles.css (all the styles in node_modules/@docusarus and in our docs site components)
  • discordapp-styles.css (all the app styles, except for the reset styles)
  • discordapp-reset.css (our own app's reset)

In the head, we load docusaurus-styles.css and discordapp-styles.css (because we want to use our components in doc pages), but in the shadow DOM, we only load discordapp-*.css (because we don't want Docusaurus' global styles to pollute the component playgrounds, which need to look exactly the same as they do in Discord).

The only issue we've faced with Shadow DOM is that some event handlers needed to be changed. For example, for a popup component (think popper, context menus, dropdowns, etc.), we had a mouseup listener on document, and it checked if popupContainer.contains(event.target). With shadow DOM, this doesn't work, because event.target is always the shadow root, so we had to change it to popupContainer.contains(event.composedPaths()[0]).

I'll report back if I see other issues. I agree that it would be great to not have to use Shadow DOM, but it sounds far worse to have to import some <Pre> component rather than just using HTML <pre>. @scope is really the answer. Could just be:

@scope (body.docusaurus) to (.docusaurus-exclude) {
  /* all docusaurus styles */
}

@slorber
Copy link
Collaborator

slorber commented Oct 5, 2022

A new interesting solution from @bluwy

Whyframe permits to easily isolate the rendering of a React component inside an iframe.

There's a Docusaurus plugin to try

@dcousineau
Copy link

dcousineau commented Feb 17, 2023

We took an approach similar to @stevenpetryk and wanted to post the rough outline for anyone else who finds this thread below.

On a side note, I think there's a possibility that a contribution to @docusaurus/theme-live-codeblock could be made to have a toggleable Shadow DOM approach, the primary coordination could be that Docusaurus's splitChunks configuration for CSS files is altered such that infima and @docusaurus css files are chunked separately, and "everything else" is in a separate well-named chunk that, if shadow dom is enabled in the live codeblock, could be pulled in whole-sale for a ... "quick and dirty" isolated codeblock experience. Will see if I can carve out some time to put together the skeleton of the idea.


In the theme that we ship, our basic approach was to tweak the splitChunks cacheGroups to isolate our component library styles:

const DEFAULT_LIVE = {
  enableShadowDOM: true,
  includeSplitChunks: c => {
    return /@package-namespace[\\/].+\.s?css/.test(c.identifier());
  }
};

module.exports = function (context, options) {
  const { /* ...snip...*/, live = DEFAULT_LIVE } = options || {};

  return {
    name: 'our-theme',
    // ...snip...
    configureWebpack(config, isServer, utils) {
      const updateConfig = {};

      if (!isServer && live.enableShadowDOM) {
        // Utilize split chunks to ensure CSS assets from Docusaurus are separated from others
        updateConfig.mergeStrategy = updateConfig.mergeStrategy ?? {};
        updateConfig.mergeStrategy['optimization.splitChunks.cacheGroups'] = 'merge';
        updateConfig.optimization = {
          splitChunks: {
            cacheGroups: {
              liveStyles: {
                name: 'styles-live',
                test: c => {
                  // Additionally include any styles that may have been pulled in by ReactLiveScope overrides
                  return /ReactLiveScope[\\/].+\.s?css/.test(c.identifier()) || live.includeSplitChunks(c);
                },
                chunks: 'all',
                // enforce: false feels weird to have, but it DOES result in the CSS outputs we need.
                // @see https://github.com/thecrypticace/webpack-css-chunks-test/blob/master/webpack.config.js
                enforce: false,
                priority: 60 // The default `styles` cacheGroup from Docusaurus is set to priority 50
              }
            }
          }
        };
      }

      return updateConfig;
    },
    // ...snip...
  };
}

And then we ejected the Playground component from @docusaurus/theme-live-codeblock into our custom theme and made some tweaks to mount the <LivePreview /> in a shadow root (there is a lot of snipping below, we have a heavily customized Playground component)

// @see https://github.com/facebook/docusaurus/blob/master/packages/docusaurus-theme-live-codeblock/src/theme/Playground/index.js
import React, { useState, useRef, useEffect, useMemo } from 'react';
import { LiveProvider, LiveEditor, LiveError, LivePreview } from 'react-live';
import useDocusaurusContext from '@docusaurus/useDocusaurusContext';
import ShadowRoot from 'react-shadow';

export default function Playground({ children, transformCode, ...props }) {
  // ...snip...
  const {
    siteConfig: {
      themeConfig: { live = {} }
    }
  } = useDocusaurusContext();

  // We assume that shadowDOM is enabled by default
  const enableShadowDOM = live?.enableShadowDOM ?? true;

  const sheets = useMemo(() => {
    if (!enableShadowDOM) return [];

    const sheets = typeof document !== 'undefined' && document?.styleSheets?.length && Array.from(document.styleSheets);
    if (!sheets) return [];

    return sheets.filter(sheet => {
      if (!sheet.href) return false;
      const sheetHref = new URL(sheet.href);
      return sheetHref.pathname.match(/styles-live/i);
    });
  });

  return (
    <div className={styles.playgroundContainer}>
      <LiveProvider /* ...snip... */ >
        <div className={styles.playgroundPreview}>
          {enableShadowDOM ? (
            <ShadowRoot.div ref={root} mode='open'>
              <LivePreview />
              {sheets.map(sheet => (
                <style type='text/css'>{Array.from(sheet.cssRules, rule => rule.cssText || '')}</style>
              ))}
            </ShadowRoot.div>
          ) : (
            <LivePreview />
          )}
          {showEditor && <LiveError />}
        </div>
        <LiveEditor className={styles.playgroundEditor} />
        {/* ...snip... */}
      </LiveProvider>
    </div>
  );
}

Note
The above approach is currently naïve especially in regards to pulling in the stylesheet for the live playground (effectively only once on mount and maybe a second time if React clears the memoization cache).
We'll be spending more effort over time improving / bullet proofing the solution but for now it works "good enough".

@gpbl
Copy link

gpbl commented Apr 16, 2023

Thanks for the feedbacks! I am still not happy with the workarounds described here.

What we need is the ability to create a blank HTML page in Docusaurus, but without any styles. We could them link this page with a standard iframe. Can we do this without forking the whole classic theme?

@slorber
Copy link
Collaborator

slorber commented Apr 19, 2023

What we need is the ability to create a blank HTML page in Docusaurus, but without any styles. We could them link this page with a standard iframe. Can we do this without forking the whole classic theme?

If you don't need SPA navigation, you can add any html file in /static/xyz.html and link to it with pathname:///xyz.html or display it inside an iframe

@slorber
Copy link
Collaborator

slorber commented Jan 30, 2024

Note to myself: cascade layers can also be a good alternative to opt-out of the style we apply by default.

This article explains how to use all: revert-layer for that purpose:
https://www.mayank.co/blog/revert-layer

gpbl added a commit to gpbl/react-day-picker that referenced this issue Jul 7, 2024
gpbl added a commit to gpbl/react-day-picker that referenced this issue Jul 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
external This issue is caused by an external dependency and not Docusaurus. proposal This issue is a proposal, usually non-trivial change
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants