build!: generate cjs and type declarations #1572

jinlee93 · 2023-03-31T08:18:08Z

Summary:

builds both cjs and esm with rollup
- cjs built as *.cjs files to /lib
- esm built as *.js files to lib
- sourcemaps also turned on
mitigated by not building declarations with rollup, and using tsc to build declarations separately into lib/types
now looks like:

Test Plan:

Wrote automated tests
CI tests / new tests are not applicable
Manually tested my changes, but I want to keep the details secret
Manually tested my changes, and here are the details:
- built and copied folder into edu_stack node_modules/@chanzuckerberg/eds
- added a debugger line to one of the used components in the es folder to make sure the esm build was being used:
- also saw that sourcemaps being built correctly
- adding debugger into a component not being used did not trigger the browser debugger, suggesting tree shaking is working
- running npm test in edu-stack passes even withtransformIgnorePatterns: ['/node_modules/(?!(@chanzuckerberg/eds))'] removed
  - passes now without tslib since that was a peerdep of @rollup/plugin-typescript

jinlee93 · 2023-03-31T08:22:32Z

tsconfig.json

    "esModuleInterop": true,
    "forceConsistentCasingInFileNames": true,
    "jsx": "react",
    "lib": ["es5", "es6", "dom"],
    "moduleResolution": "node",
    "noUnusedLocals": true,
-    "outDir": "lib",


No longer using tsc to build

jinlee93 · 2023-03-31T08:22:41Z

tsconfig.json

@@ -4,13 +4,13 @@
    "allowJs": false,
    "allowSyntheticDefaultImports": true,
    "declaration": true,
+    "declarationDir": "lib/types",


builds the declaration files to this folder

jinlee93 · 2023-03-31T08:23:25Z

rollup.config.mjs

+         */
+        declaration: false,
+        declarationDir: undefined,
+      },


There's already an issue raised in the plugin repo rollup/plugins#1230 (comment)

jinlee93 · 2023-03-31T08:24:17Z

rollup.config.mjs

+      format: 'cjs',
+      preserveModules: true,
+      preserveModulesRoot: 'src',
+      sourcemap: true,


We can turn sourcemaps off, not sure if anyone will take a look but also wouldn't hurt to have

jinlee93 · 2023-03-31T08:24:31Z

package.json

@@ -90,8 +93,6 @@
    "react-popper": "^2.3.0",
    "react-portal": "^4.2.2"
  },
-  "entry": "lib/index.js",


Not sure what this line did?

No idea. Probs something in the BM template that doesn't do anything.

codecov · 2023-03-31T08:26:07Z

Codecov Report

Merging #1572 (2d05ebd) into ah-ssr-build (a917da5) will not change coverage.
The diff coverage is n/a.

❗ Current head 2d05ebd differs from pull request most recent head 90b865b. Consider uploading reports for the commit 90b865b to get more accurate results

@@              Coverage Diff              @@
##           ah-ssr-build    #1572   +/-   ##
=============================================
  Coverage         91.63%   91.63%           
=============================================
  Files               279      279           
  Lines              4269     4269           
  Branches            775      775           
=============================================
  Hits               3912     3912           
  Misses              331      331           
  Partials             26       26

Impacted Files	Coverage Δ
src/index.ts	`100.00% <ø> (ø)`

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

ahuth · 2023-03-31T13:51:32Z

package.json

+  "main": "lib/cjs/index.js",
+  "module": "lib/es/index.js",


These will certainly work 👍

Although in the future we may want to switch over to conditional exports.

Per https://nodejs.org/api/packages.html#dual-commonjses-module-packages (and other places in the docs), I think exports is preferred now (although main/module is still common).

ahuth

Nice 🤘

jinlee93 · 2023-03-31T21:41:48Z

package.json

+    "./tailwind.config.mjs": {
+      "import": "./tailwind.config.mjs"
+    }
+  },


@ahuth Not sure if I'm using conditional exports right but seems that we have to explicitly state which files are allowed to be exported

Yeah, that's true. I kind of forgot that there was more than just the index.js files (such as fonts, css, tailwind, etc).

Does this seem like it's too cumbersome, or is it okay?

It's fine unless consumers want to use some of the internal components or the utils, which we haven't come across yet

jinlee93 · 2023-03-31T21:42:36Z

rollup.config.mjs

+      preserveModules: true,
+      preserveModulesRoot: 'src',
+      sourcemap: true,
+      entryFileNames: '[name].cjs',


This builds the cjs files using .cjs extension into the same directories, no longer requiring separate es and cjs directories... I prefer it this way but we can always go back to separate directories

Either way seems reasonable, so let's go with your preference.

jinlee93 · 2023-03-31T21:44:36Z

tailwind.config.mjs

@@ -64,4 +66,4 @@ export default {
      xxl: '1400px',
    },
  },
-} satisfies Config;
+};


@ahuth I converted this into an mjs file so that it can be used without processing in pilots, still getting typing with the jsdoc @type tag

I think it's better to keep this TS, so that it's easier for the pilots to write their tailwind files in TS.

Meaning, the pilot swill have to process their tailwind.config.ts anyway, so it's fine for that TS file to import another TS file (the EDS tailwind config).

Makes sense to me, reverted to TS

jinlee93 · 2023-03-31T21:48:45Z

package.json

+    "./index.css": {
+      "import": "./lib/index.css",
+      "require": "./lib/index.css"
+    },
+    "./fonts.css": {
+      "import": "./lib/tokens/fonts.css",
+      "require": "./lib/tokens/fonts.css"
+    },


I short handed some of these styles exports so it's less unwieldy (i.e. import '@chanzuckerberg/eds/fonts.css and not import @chanzuckerberg/eds/lib/tokens/fonts.css) ... it IS breaking but the whole build feature will be breaking so might as well include it... lmk if we shouldn't break this since cost is pretty low

jinlee93 · 2023-03-31T21:59:26Z

Still getting the test errors in edu-stack

with conditional exports

jinlee93 · 2023-04-03T20:18:14Z

tsconfig.json

    "resolveJsonModule": true,
    "skipLibCheck": true,
    "strict": true,
    "suppressImplicitAnyIndexErrors": true,
-    "target": "es6"
+    "target": "es2018"


Target is being bumped to remove tslib usage in running rollup build with @rollup/plugin-typescript. tslib was used for __rest utility which is the spread object functionality that comes with es2018. tslib is being grepped for as a build test

jinlee93 · 2023-04-03T20:20:47Z

.github/workflows/test.yml

+          then
+            echo "tslib found in lib"
+            exit 1
+          fi


tested failing with es2017 https://github.com/chanzuckerberg/edu-design-system/actions/runs/4600946862/jobs/8128192461

* 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 <ahuth@chanzuckerberg.com>

## [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)

build: generate cjs and esm, separate types build

e4c15c4

jinlee93 requested a review from a team March 31, 2023 08:18

build: add sourcemaps and add declaration back to tsconfig

45f2ba5

jinlee93 force-pushed the jlee/alsobuildcjs branch from 22dab1b to 45f2ba5 Compare March 31, 2023 08:20

jinlee93 commented Mar 31, 2023

View reviewed changes

jinlee93 added the do not merge PRs that should not be merged (even if the build is green or there are approvals) label Mar 31, 2023

ahuth reviewed Mar 31, 2023

View reviewed changes

ahuth approved these changes Mar 31, 2023

View reviewed changes

Jin Lee added 2 commits March 31, 2023 13:47

build: cjs and js in the same output

0a3c65d

build: add conditional exports

8a7c7a6

jinlee93 changed the title ~~build: generate cjs and type declarations~~ build!: generate cjs and type declarations Mar 31, 2023

jinlee93 commented Mar 31, 2023

View reviewed changes

build: remove build types script

7d1b939

jinlee93 requested a review from ahuth March 31, 2023 21:59

Jin Lee added 2 commits March 31, 2023 16:58

refactor: revert tw config to ts

2d05ebd

build: bump target to es2018 and grep for tslib

90b865b

jinlee93 removed the do not merge PRs that should not be merged (even if the build is green or there are approvals) label Apr 3, 2023

jinlee93 commented Apr 3, 2023

View reviewed changes

ahuth approved these changes Apr 3, 2023

View reviewed changes

jinlee93 merged commit 5efbba3 into ah-ssr-build Apr 3, 2023
4 checks passed

jinlee93 deleted the jlee/alsobuildcjs branch April 3, 2023 23:58

jinlee93 mentioned this pull request Apr 3, 2023

build!: use rollup #1555

Merged

10 tasks

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

build!: generate cjs and type declarations #1572

build!: generate cjs and type declarations #1572

jinlee93 commented Mar 31, 2023 •

edited

jinlee93 Mar 31, 2023

jinlee93 Mar 31, 2023

jinlee93 Mar 31, 2023

jinlee93 Mar 31, 2023

jinlee93 Mar 31, 2023

ahuth Mar 31, 2023

codecov bot commented Mar 31, 2023 •

edited

ahuth Mar 31, 2023

ahuth left a comment

jinlee93 Mar 31, 2023

ahuth Mar 31, 2023

jinlee93 Mar 31, 2023

jinlee93 Mar 31, 2023 •

edited

ahuth Mar 31, 2023

jinlee93 Mar 31, 2023 •

edited

ahuth Mar 31, 2023 •

edited

jinlee93 Mar 31, 2023

jinlee93 Mar 31, 2023

jinlee93 commented Mar 31, 2023

jinlee93 Apr 3, 2023 •

edited

jinlee93 Apr 3, 2023

build!: generate cjs and type declarations #1572

build!: generate cjs and type declarations #1572

Conversation

jinlee93 commented Mar 31, 2023 • edited

Summary:

Test Plan:

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

Choose a reason for hiding this comment

codecov bot commented Mar 31, 2023 • edited

Codecov Report

Choose a reason for hiding this comment

ahuth left a 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

jinlee93 Mar 31, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jinlee93 Mar 31, 2023 • edited

Choose a reason for hiding this comment

ahuth Mar 31, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jinlee93 commented Mar 31, 2023

jinlee93 Apr 3, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jinlee93 commented Mar 31, 2023 •

edited

codecov bot commented Mar 31, 2023 •

edited

jinlee93 Mar 31, 2023 •

edited

jinlee93 Mar 31, 2023 •

edited

ahuth Mar 31, 2023 •

edited

jinlee93 Apr 3, 2023 •

edited