Publish bundles instead of modules, use babel only for transpilation #2214

nstepien · 2020-11-07T17:00:30Z

Pros:

Babel's output is better and less buggy than TS's.
Can publish with the new React runtime sooner.
More flexibility, can use fancy babel/rollup plugins:
- clsx calls are optimized, users don't need to add an extra babel plugin.
- We could do try doing css-in-js.
- I'm thinking about perhaps adding a light terser config, or perhaps try the closure compiler. Not sure if it's worth it.
- https://babeljs.io/docs/en/babel-plugin-transform-react-constant-elements 👀
Smaller npm footprint:
- No mixed bundles/modules+sourcemaps in published files.
- Code that's not exported or unused by exports will be tree-shaken.

We still use tsc to output declaration files.

nstepien · 2020-11-07T17:06:21Z

.eslintrc.js

@@ -483,7 +483,7 @@ const rules = {
  '@typescript-eslint/consistent-indexed-object-style': 1,
  '@typescript-eslint/consistent-type-assertions': [2, { assertionStyle: 'as', objectLiteralTypeAssertions: 'never' }],
  '@typescript-eslint/consistent-type-definitions': [1, 'interface'],
-  '@typescript-eslint/consistent-type-imports': [1, { prefer: 'no-type-imports' }],
+  '@typescript-eslint/consistent-type-imports': 1,


I've tweaked this so we'll now use type imports, this is to help babel elide type imports, otherwise it cannot always know to remove some imports, especially in files like src/index.ts.

nstepien · 2020-11-07T17:08:26Z

babel.config.json

+        "@babel/proposal-nullish-coalescing-operator",
+        "@babel/proposal-optional-chaining"


Fun fact: with our browserslist config, and our code, aside from these two transform plugins, @babel/preset-env does nothing.

nstepien · 2020-11-07T17:08:49Z

babel.config.json

+        "@babel/proposal-optional-chaining"
+      ]
+    }],
+    ["@babel/react", { "useSpread": true }],


I'll change this to use the new runtime in #2184

nstepien · 2020-11-07T17:09:33Z

babel.config.json

+      ]
+    }],
+    ["@babel/react", { "useSpread": true }],
+    "@babel/typescript"
  ],
  "plugins": [
    ["@babel/transform-runtime", { "useESModules": true }],


We might have to add @babel/runtime in our dependencies in the future, but currently no helpers get imported in our bundles.
useESModules should also be set to false for the cjs bundle.

nstepien · 2020-11-07T17:11:51Z

rollup.config.mjs

-    'react',
-    'react-dom'
-  ],
+  external: id => !id.startsWith('.') && !isAbsolute(id),


This will mark all non-relative imports as external, in other words all npm dependencies.

amanmahajan7

A few questions. LGTM

amanmahajan7 · 2020-11-09T19:55:19Z

babel.config.json

@@ -1,11 +1,18 @@
 {
  "presets": [
    ["@babel/env", {
+      "loose": true,


Should we be using loose?

Original code:

ref.current?.focus({ preventScroll: true });

With loose enabled:

(_ref$current = ref.current) == null ? void 0 : _ref$current.focus({

With loose disabled:

(_ref$current = ref.current) === null || _ref$current === void 0 ? void 0 : _ref$current.focus({

In non-loose mode, the value is explicitely checked for both null and undefined, as it needs to work with document.all, but we don't use document.all anywhere so it's safe.

It might matter in the future for new js syntax features, but loose will only break edge cases so I think it's fine.

amanmahajan7 · 2020-11-09T20:00:08Z

tsconfig.all.json

@@ -1,6 +1,7 @@
 {
  "extends": "./tsconfig.json",
  "compilerOptions": {
+    "emitDeclarationOnly": false,


Do we need this if we have noEmit?

Yes unfortunately.

> [email protected] typecheck > tsc -p tsconfig.all.json tsconfig.all.json:4:5 - error TS5053: Option 'emitDeclarationOnly' cannot be specified with option 'noEmit'. 4 "noEmit": true ~~~~~~~~ Found 1 error.

amanmahajan7 · 2020-11-09T20:05:11Z

rollup.config.mjs

-    nodeResolve()
+    babel({
+      babelHelpers: 'runtime',
+      skipPreflightCheck: true,


Not sure if it is worth enabling it? Does it slow down the build by much?

It builds in 1s in both cases, but I ran into issues before so I had to add this. I'll remove it now.

Publish bundles instead of modules, use babel only for transpilation

38bfb21

nstepien self-assigned this Nov 7, 2020

nstepien commented Nov 7, 2020

View reviewed changes

Fix jest

5985acc

nstepien marked this pull request as ready for review November 7, 2020 17:25

nstepien requested a review from amanmahajan7 November 7, 2020 17:25

nstepien added 6 commits November 7, 2020 17:43

ocd

5850ca7

Strip comments from bundles

e0115ac

preset-env: enable loose transformation

3eda9ac

Merge branch 'canary' into babundle

e7b653c

Fix lint

ae3b358

Merge branch 'canary' into babundle

3f73b29

amanmahajan7 reviewed Nov 9, 2020

View reviewed changes

rm skipPreflightCheck

eec804d

amanmahajan7 approved these changes Nov 9, 2020

View reviewed changes

nstepien merged commit 7b015eb into canary Nov 9, 2020

nstepien deleted the babundle branch November 9, 2020 20:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Publish bundles instead of modules, use babel only for transpilation #2214

Publish bundles instead of modules, use babel only for transpilation #2214

nstepien commented Nov 7, 2020 •

edited

Loading

nstepien Nov 7, 2020 •

edited

Loading

nstepien Nov 7, 2020

nstepien Nov 7, 2020

nstepien Nov 7, 2020 •

edited

Loading

nstepien Nov 7, 2020

amanmahajan7 left a comment

amanmahajan7 Nov 9, 2020

nstepien Nov 9, 2020

amanmahajan7 Nov 9, 2020

nstepien Nov 9, 2020

amanmahajan7 Nov 9, 2020

nstepien Nov 9, 2020

		"@babel/proposal-nullish-coalescing-operator",
		"@babel/proposal-optional-chaining"

Publish bundles instead of modules, use babel only for transpilation #2214

Publish bundles instead of modules, use babel only for transpilation #2214

Conversation

nstepien commented Nov 7, 2020 • edited Loading

nstepien Nov 7, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nstepien Nov 7, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

amanmahajan7 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nstepien commented Nov 7, 2020 •

edited

Loading

nstepien Nov 7, 2020 •

edited

Loading

nstepien Nov 7, 2020 •

edited

Loading