refactor: remove unused commonjs plugin #1582

jinlee93 · 2023-04-04T23:43:20Z

Summary:

removes unused plugin @rollup/plugin-commonjs

Test Plan:

CI tests / new tests are not applicable

jinlee93 · 2023-04-04T23:43:54Z

CHANGELOG.md

@@ -2,6 +2,8 @@

 All notable changes to this project will be documented in this file. See [standard-version](https://github.com/conventional-changelog/standard-version) for commit guidelines.

+## [12.0.0-alpha.1](https://github.com/chanzuckerberg/edu-design-system/compare/v12.0.0-alpha.0...v12.0.0-alpha.1) (2023-04-04)
+


Oop alpha publish changelogs snuck in here but shouldn't matter

I looked back and saw a few other older builds included alpha stuff in the change log. This shouldn't interfere with any releases, but makes the change log slightly weird

jinlee93 · 2023-04-04T23:44:23Z

package.json

@@ -109,7 +109,6 @@
    "@commitlint/cli": "^17.5.1",
    "@commitlint/config-conventional": "^17.4.4",
    "@geometricpanda/storybook-addon-badges": "^1.1.1",
-    "@rollup/plugin-commonjs": "^24.0.1",


Unused plugin, tsconfig handles for us

codecov · 2023-04-04T23:45:06Z

Codecov Report

Merging #1582 (3910985) into ah-ssr-build (3cf44ff) will not change coverage.
The diff coverage is n/a.

@@              Coverage Diff              @@
##           ah-ssr-build    #1582   +/-   ##
=============================================
  Coverage         91.39%   91.39%           
=============================================
  Files               279      279           
  Lines              4147     4147           
  Branches            776      776           
=============================================
  Hits               3790     3790           
- Misses              331      355   +24     
+ Partials             26        2   -24

see 11 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

jinlee93 · 2023-04-04T23:46:23Z

All changed component files are just converting import clsx from "clsx" to import { clsx } from "clsx". The other changes are alpha publishes and removing unused plugin

ahuth · 2023-04-05T11:03:37Z

.storybook/components/TokenSpecimen/TokenSpecimen.tsx

@@ -1,4 +1,4 @@
-import clsx from 'clsx';
+import { clsx } from 'clsx';


Weird. What error gets thrown with default export + cjs builds?

Whether it's named or not shouldn't matter.

Same question; why is clsx having this issue?

@booc0mtaco paired on this and saw that <Link> is being used in a cjs build from remix-utils which requires clsx, which I thought would pull the main build of clsx according to its package.json but instead uses the module build. Removing that module line in clsx's package json in edu-stack allows it to resolve to the main build instead and works fine with the default export, implying this issue could arise from import/require resolution within edu-stack

Ahhhhhh, the link to remix-utils might be a clue.

I wonder if remix-utils/Link is importing the ESM version of EDS somehow, which then tries to load the ESM versino of clsx? 🤔

remix-utils/Link is using the CJS version, but
How about building both cjs and esm for remix-utils and using conditional exports for it?: aka
This works on edu-stack

This reverts commit d58182b.

booc0mtaco

simpler PR :) I am assuming this plugin came thru previous work on the feature branch and we are essentially reverting it from there?

jinlee93 · 2023-04-05T22:25:25Z

simpler PR :) I am assuming this plugin came thru previous work on the feature branch and we are essentially reverting it from there?

Yup!

jinlee93 · 2023-04-10T17:21:58Z

Going to land this for now since now unrelated to esm / cjs build

* build: use rollup to build JS and styles - To better support server-side rendering - By producing a single .css file - Instead of delegating css module processing to consuming apps * refactor(rollup): remove spritemap and commonjs from config * build: remove styles build from main build command * refactor: import css variables into index * build(rollup): mark dependencies as external (#1558) * build(rollup): mark dependencies as external * build(rollup): use node resolve plugin * docs(build): explain rollup external option * build!: generate cjs and type declarations (#1572) * build: generate cjs and esm, separate types build * build: add sourcemaps and add declaration back to tsconfig * build: cjs and js in the same output * build: add conditional exports * build: remove build types script * refactor: revert tw config to ts * build: bump target to es2018 and grep for tslib * chore(release): 12.0.0-alpha.0 * refactor: dry css exports * build!: remove some tokens from build (#1581) * refactor: remove references to unexported files * build!: no longer include some tokens in build * refactor: remove unused commonjs plugin (#1582) * chore(release): 12.0.0-alpha.1 * refactor: use named import for clsx * chore: remove rollup plugin commonjs * Revert "refactor: use named import for clsx" This reverts commit d58182b. * docs: explicate tslib grep * fix: no need for ts expect error --------- Co-authored-by: Andrew Huth <[email protected]>

## [12.0.0](v11.1.1...v12.0.0) (2023-04-17) ### ⚠ BREAKING CHANGES * use rollup (#1555) ### Features * export some subcomponents ([#1579](#1579)) ([2857ae4](2857ae4)) * **TextareaField:** add character length counter ([#1580](#1580)) ([ff6226f](ff6226f)) ### Bug Fixes * restore check for undefined any types ([#1585](#1585)) ([c7fae07](c7fae07)) * **Skeleton:** mark .Rect as deprecated ([#1586](#1586)) ([405f81b](405f81b)) * sync typography presets to documentation ([#1592](#1592)) ([b56eadb](b56eadb)) * **typography:** add missing eds-font-size-20 ([#1591](#1591)) ([de5dd03](de5dd03)) ### build * use rollup ([#1555](#1555)) ([d794696](d794696)), closes [#1558](#1558) [#1572](#1572) [#1581](#1581) [#1582](#1582)

Jin Lee added 2 commits April 4, 2023 15:52

chore(release): 12.0.0-alpha.1

d093b15

refactor: use named import for clsx

d58182b

jinlee93 requested a review from a team April 4, 2023 23:43

jinlee93 commented Apr 4, 2023

View reviewed changes

chore: remove rollup plugin commonjs

56b590d

jinlee93 force-pushed the jlee/useNamedClsx branch from 2d7e47b to 56b590d Compare April 4, 2023 23:45

ahuth reviewed Apr 5, 2023

View reviewed changes

Revert "refactor: use named import for clsx"

3910985

This reverts commit d58182b.

jinlee93 changed the title ~~fix: use named clsx and remove unused plugin~~ fix: remove unused commonjs plugin Apr 5, 2023

jinlee93 changed the title ~~fix: remove unused commonjs plugin~~ refactor: remove unused commonjs plugin Apr 5, 2023

booc0mtaco approved these changes Apr 5, 2023

View reviewed changes

jinlee93 merged commit 54db847 into ah-ssr-build Apr 10, 2023

jinlee93 deleted the jlee/useNamedClsx branch April 10, 2023 17:22

booc0mtaco mentioned this pull request Apr 17, 2023

Release v12.0.0 #1593

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor: remove unused commonjs plugin #1582

refactor: remove unused commonjs plugin #1582

jinlee93 commented Apr 4, 2023 •

edited

Loading

jinlee93 Apr 4, 2023

booc0mtaco Apr 5, 2023

jinlee93 Apr 4, 2023

codecov bot commented Apr 4, 2023 •

edited

Loading

jinlee93 commented Apr 4, 2023

ahuth Apr 5, 2023

booc0mtaco Apr 5, 2023

jinlee93 Apr 5, 2023

ahuth Apr 5, 2023

jinlee93 Apr 5, 2023

booc0mtaco left a comment

jinlee93 commented Apr 5, 2023

jinlee93 commented Apr 10, 2023

		@@ -2,6 +2,8 @@

		All notable changes to this project will be documented in this file. See [standard-version](https://github.com/conventional-changelog/standard-version) for commit guidelines.

		## [12.0.0-alpha.1](https://github.com/chanzuckerberg/edu-design-system/compare/v12.0.0-alpha.0...v12.0.0-alpha.1) (2023-04-04)

		@@ -1,4 +1,4 @@
		import clsx from 'clsx';
		import { clsx } from 'clsx';

refactor: remove unused commonjs plugin #1582

refactor: remove unused commonjs plugin #1582

Conversation

jinlee93 commented Apr 4, 2023 • edited Loading

Summary:

Test Plan:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Apr 4, 2023 • edited Loading

Codecov Report

jinlee93 commented Apr 4, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

booc0mtaco left a comment

Choose a reason for hiding this comment

jinlee93 commented Apr 5, 2023

jinlee93 commented Apr 10, 2023

jinlee93 commented Apr 4, 2023 •

edited

Loading

codecov bot commented Apr 4, 2023 •

edited

Loading