Skip to content

Commit

Permalink
fix(react): correctly mark dependencies as external (#2479)
Browse files Browse the repository at this point in the history
This ensures Accordion works when consumers have enabled tree-shaking.

Because we didn't mark all our dependencies as external,
`import "@u-elements/u-details"` was tree-shaked when
`@digdir/designsystemet-react`
was used in an application with tree-shaking enabled.

This happened because we bundled several dependencies (including this one)
with our library, and our library has `"sideEffects": false`, implying that
any side-effect imports that happened within our library folder was safe
to remove.

This was fixed by correctly marking all our dependencies as external, in
which case the application's build will correctly identify `"@u-elements/u-details"`
as having side-effects and thus not not remove the import.

Closes #2477
  • Loading branch information
unekinn authored Sep 20, 2024
1 parent dacf6f0 commit 047c9c1
Show file tree
Hide file tree
Showing 12 changed files with 45 additions and 23 deletions.
5 changes: 5 additions & 0 deletions .changeset/brave-months-shop.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@digdir/designsystemet-react': patch
---

Correctly mark dependencies as external. This ensures Accordion works when consumers have enabled tree-shaking.
3 changes: 3 additions & 0 deletions .github/workflows/checks-packages.yml
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@ jobs:
- uses: ./.github/actions/gh-setup
- name: Build
run: yarn build
- name: Check that we don't bundle node_modules
shell: bash
run: if [ -d packages/react/dist/esm/node_modules ]; then exit 1; fi
- name: Types
run: yarn types:react
- name: Test
Expand Down
1 change: 1 addition & 0 deletions apps/_components/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
"@digdir/designsystemet-react": "workspace:^",
"@navikt/aksel-icons": "^6.14.0",
"@types/react-syntax-highlighter": "^15.5.13",
"clsx": "^2.1.1",
"react-syntax-highlighter": "^15.5.0"
},
"devDependencies": {
Expand Down
1 change: 1 addition & 0 deletions apps/storefront/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
"@next/mdx": "^14.2.5",
"@repo/components": "workspace:^",
"@vercel/analytics": "^1.3.1",
"clsx": "^2.1.1",
"hastscript": "^9.0.0",
"next": "^14.2.5",
"react": "^18.3.1",
Expand Down
1 change: 1 addition & 0 deletions apps/storybook/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
"license": "ISC",
"description": "",
"dependencies": {
"clsx": "^2.1.1",
"react": "^18.3.1",
"react-dom": "^18.3.1"
},
Expand Down
2 changes: 1 addition & 1 deletion apps/theme/components/Previews/Components/Components.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -398,7 +398,7 @@ export const Components = () => {
<Dropdown.Item>Spansk</Dropdown.Item>
<Dropdown.Item>Fransk</Dropdown.Item>
</Dropdown>
<HelpText title='Du har ikke valgt språk'>
<HelpText aria-label='Du har ikke valgt språk'>
Velg språk for å endre innholdet på siden
</HelpText>
</Dropdown.Context>
Expand Down
1 change: 1 addition & 0 deletions apps/theme/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
"@react-awesome/use-click-outside": "^0.0.3",
"@repo/components": "workspace:^",
"chroma-js": "^2.6.0",
"clsx": "^2.1.1",
"next": "^14.2.5",
"react": "^18.3.1",
"react-color": "^2.19.3",
Expand Down
1 change: 0 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@
"@vitejs/plugin-react-swc": "^3.7.0",
"@vitest/coverage-v8": "^2.0.5",
"@vitest/expect": "^2.0.5",
"clsx": "^2.1.1",
"copyfiles": "^2.4.1",
"prettier": "^3.3.3",
"stylelint": "^16.8.1",
Expand Down
4 changes: 2 additions & 2 deletions packages/react/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,8 @@
"@navikt/aksel-icons": "^6.14.0",
"@radix-ui/react-slot": "^1.1.0",
"@tanstack/react-virtual": "^3.10.7",
"@u-elements/u-details": "^0.0.5"
"@u-elements/u-details": "^0.0.5",
"clsx": "^2.1.1"
},
"devDependencies": {
"@rollup/plugin-commonjs": "^26.0.1",
Expand All @@ -55,7 +56,6 @@
"react-dom": "^18.3.1",
"rimraf": "^6.0.1",
"rollup": "^4.20.0",
"rollup-plugin-peer-deps-external": "^2.2.4",
"typescript": "^5.5.4"
}
}
31 changes: 23 additions & 8 deletions packages/react/rollup.config.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,26 @@
import commonjs from '@rollup/plugin-commonjs';
import resolve from '@rollup/plugin-node-resolve';
import peerDepsExternal from 'rollup-plugin-peer-deps-external';
import pkg from './package.json';

// These are dependencies, but are not in our package.json
const implicitDependencies = [
'@digdir/designsystemet-theme',
'@digdir/designsystemet-css',
'@digdir/design-system-tokens',
];

const dependencies = Object.keys({
...implicitDependencies,
...pkg.dependencies,
...pkg.peerDependencies,
});

/*
Regexes to correctly mark submodules from dependencies as being external
*/
const dependenciesSubmodules = dependencies.map(
(dep) => new RegExp(`^${dep}/`),
);

export default [
{
Expand All @@ -21,12 +41,7 @@ export default [
preserveModulesRoot: 'tsc-build',
},
],
external: [
/@digdir\/designsystemet-theme/,
/@digdir\/designsystemet-css/,
/@digdir\/design-system-tokens/,
/@navikt\/aksel-icons/,
],
plugins: [peerDepsExternal(), resolve(), commonjs()],
external: [...dependencies, ...dependenciesSubmodules],
plugins: [resolve(), commonjs()],
},
];
1 change: 1 addition & 0 deletions plugins/figma-plugin/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
"@digdir/designsystemet-css": "workspace:^",
"@digdir/designsystemet-react": "workspace:^",
"@digdir/designsystemet-theme": "workspace:^",
"clsx": "^2.1.1",
"react": "^18.3.1",
"react-dom": "^18.3.1",
"react-router-dom": "^6.26.0"
Expand Down
17 changes: 6 additions & 11 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -2926,6 +2926,7 @@ __metadata:
"@storybook/types": "npm:^8.2.9"
"@types/react": "npm:18.3.3"
"@types/react-dom": "npm:^18.3.0"
clsx: "npm:^2.1.1"
react: "npm:^18.3.1"
react-dom: "npm:^18.3.1"
remark-gfm: "npm:^4.0.0"
Expand Down Expand Up @@ -2975,13 +2976,13 @@ __metadata:
"@testing-library/react": "npm:^16.0.0"
"@testing-library/user-event": "npm:^14.5.2"
"@u-elements/u-details": "npm:^0.0.5"
clsx: "npm:^2.1.1"
copyfiles: "npm:^2.4.1"
jsdom: "npm:^24.1.1"
react: "npm:^18.3.1"
react-dom: "npm:^18.3.1"
rimraf: "npm:^6.0.1"
rollup: "npm:^4.20.0"
rollup-plugin-peer-deps-external: "npm:^2.2.4"
typescript: "npm:^5.5.4"
peerDependencies:
react: ">=18.3.1"
Expand Down Expand Up @@ -4075,6 +4076,7 @@ __metadata:
"@digdir/designsystemet-react": "workspace:^"
"@navikt/aksel-icons": "npm:^6.14.0"
"@types/react-syntax-highlighter": "npm:^15.5.13"
clsx: "npm:^2.1.1"
react-syntax-highlighter: "npm:^15.5.0"
typescript: "npm:^5.5.4"
peerDependencies:
Expand Down Expand Up @@ -9189,6 +9191,7 @@ __metadata:
"@types/react": "npm:^18.3.3"
"@types/react-dom": "npm:^18.3.0"
"@vitejs/plugin-react": "npm:^4.3.1"
clsx: "npm:^2.1.1"
npm-run-all2: "npm:^6.2.2"
postcss: "npm:^8.4.41"
react: "npm:^18.3.1"
Expand Down Expand Up @@ -14980,15 +14983,6 @@ __metadata:
languageName: node
linkType: hard

"rollup-plugin-peer-deps-external@npm:^2.2.4":
version: 2.2.4
resolution: "rollup-plugin-peer-deps-external@npm:2.2.4"
peerDependencies:
rollup: "*"
checksum: 10/49b7b3f5ca14550146249b5fd86043bffb82a2c40750c80a1845c3b694fadde58435bae0c912ad8cb1c1ef6c1f8ad64914153793391cb13d52e2bc3aa2ac61e2
languageName: node
linkType: hard

"rollup@npm:^4.13.0":
version: 4.14.0
resolution: "rollup@npm:4.14.0"
Expand Down Expand Up @@ -15126,7 +15120,6 @@ __metadata:
"@vitejs/plugin-react-swc": "npm:^3.7.0"
"@vitest/coverage-v8": "npm:^2.0.5"
"@vitest/expect": "npm:^2.0.5"
clsx: "npm:^2.1.1"
copyfiles: "npm:^2.4.1"
prettier: "npm:^3.3.3"
stylelint: "npm:^16.8.1"
Expand Down Expand Up @@ -15685,6 +15678,7 @@ __metadata:
"@types/react": "npm:^18.3.3"
"@types/react-dom": "npm:^18.3.0"
"@vercel/analytics": "npm:^1.3.1"
clsx: "npm:^2.1.1"
hastscript: "npm:^9.0.0"
next: "npm:^14.2.5"
react: "npm:^18.3.1"
Expand Down Expand Up @@ -16285,6 +16279,7 @@ __metadata:
"@types/react-color": "npm:^3.0.12"
"@types/react-dom": "npm:^18.3.0"
chroma-js: "npm:^2.6.0"
clsx: "npm:^2.1.1"
next: "npm:^14.2.5"
react: "npm:^18.3.1"
react-color: "npm:^2.19.3"
Expand Down

0 comments on commit 047c9c1

Please sign in to comment.