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.

Sign up for GitHub

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

refactor: mark a few client-side packages as side-effect-free #7085

Merged
merged 10 commits into from
Apr 9, 2022
Merged

refactor: mark a few client-side packages as side-effect-free #7085

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
2347564
refactor: mark a few client-side packages as side-effect-free
Josh-Cena Apr 1, 2022
913713c
fix
Josh-Cena Apr 1, 2022
1e34d72
fix again
Josh-Cena Apr 1, 2022
aa3a671
fix...
Josh-Cena Apr 1, 2022
b0dec03
revert sideeffect
Josh-Cena Apr 1, 2022
5954f0b
revert
Josh-Cena Apr 1, 2022
306f5a8
fix again...
Josh-Cena Apr 1, 2022
06e43aa
properly fix
Josh-Cena Apr 1, 2022
4f6fd78
fix
Josh-Cena Apr 2, 2022
621000a
properly fix
Josh-Cena Apr 8, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/docusaurus-plugin-content-docs/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
"version": "2.0.0-beta.18",
"description": "Docs plugin for Docusaurus.",
"main": "lib/index.js",
"sideEffects": false,
"exports": {
"./client": "./lib/client/index.js",
"./server": "./lib/server-export.js",
Expand Down
2 changes: 2 additions & 0 deletions packages/docusaurus-theme-classic/src/theme/Details/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ import React from 'react';
import clsx from 'clsx';
import {Details as DetailsGeneric} from '@docusaurus/theme-common';
import type {Props} from '@theme/Details';
// Ensure that the default details style is properly overridden
import '@docusaurus/theme-common/lib/components/Details/styles.module.css';
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We unfortunately have to do this, to make theme-common tree-shakeable at all. Maybe we should have different exports fields for all side-effect-ful exports (like @docusaurus/theme-common/Details), or simply declare this as the official way to acquire default styles for Details?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤷‍♂️ if we do this maybe we should remove the css module import in theme common?

Note useKeyboardNavigation also imports css

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aye, but that one's not impacted by CSS insertion order, so as long as it is loaded (which, well, means useKeyboardNavigation.ts is used), it works properly.

My idea is that we should not export Details from src/index.tsx, but create a separate exports entry point instead.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What I understand is that the CSS order problem is related to theme/Details vs theme-common/Details right?

This kind of problem is going to be solved soon once CSS cascade layers support improves (already quite good https://caniuse.com/css-cascade-layers)

@layer infima, theme-common, theme, site;

In the meantime maybe we could just add !important to the 4 rules that override theme-common?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mmm, you can see I obviously don't like !important because that makes site customizations harder, if not impossible...

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes agree, your solution is probably a better temporary workaround

import styles from './styles.module.css';

// Should we have a custom details/summary comp in Infima instead of reusing
Expand Down
3 changes: 3 additions & 0 deletions packages/docusaurus-theme-common/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@
"description": "Common code for Docusaurus themes.",
"main": "./lib/index.js",
"types": "./lib/index.d.ts",
"sideEffects": [
"lib/components/Details/*"
Josh-Cena marked this conversation as resolved.
Show resolved Hide resolved
],
"scripts": {
"build": "node copyUntypedFiles.mjs && tsc",
"watch": "node copyUntypedFiles.mjs && tsc --watch"
Expand Down
3 changes: 3 additions & 0 deletions packages/docusaurus-theme-live-codeblock/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@
"description": "Docusaurus live code block component.",
"main": "lib/index.js",
"types": "src/theme-live-codeblock.d.ts",
"sideEffects": [
"lib/theme/Playground/*"
],
"publishConfig": {
"access": "public"
},
Expand Down
3 changes: 3 additions & 0 deletions packages/docusaurus-theme-search-algolia/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@
"version": "2.0.0-beta.18",
"description": "Algolia search component for Docusaurus.",
"main": "lib/index.js",
"sideEffects": [
"*.css"
],
"exports": {
"./client": "./lib/client/index.js",
".": "./lib/index.js"
Expand Down
1 change: 1 addition & 0 deletions packages/docusaurus-utils-common/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
"description": "Common (Node/Browser) utility functions for Docusaurus packages.",
"main": "./lib/index.js",
"types": "./lib/index.d.ts",
"sideEffects": false,
"scripts": {
"build": "tsc",
"watch": "tsc --watch"
Expand Down