Add bundling (rollup) #34

dferber90 · 2018-09-07T14:39:46Z

Attempt número dos (after #30 failed to produce a treeshakeable build).

So far we were shipping untranspiled/unbundled code. This meant that all consumers of UI-Kit had to have the right webpack loaders in place in order to consume UI-Kit, which is quite bad of course. If we were to update any of our loaders, all consumers would have had to update them as well.

This PR makes it so that the UI-Kit ships as a standalone library, which can be consumed by any project which has the peerDependencies like react and react-dom of UI-Kit installed as regular dependencies.

Usage

The available commands were moved around a bit. The main updates are:

yarn start: an alias for yarn storybook:start
yarn build: New command to build the ui-kit for production to dist
yarn storybook:start: Starts the storybook locally
yarn storybook:build: Builds storybook for production

Consumers

Consumers of UI-Kit can use it by importing components from the main export:

import { PrimaryButton } from '@commercetools-frontend/ui-kit'

The main export will serve all components.

Publishing

The build gets put into the dist folder. As we don't want to force consumers to use ugly imports containing dist like @commercetools-frontend/ui-kit/dist/materials, we are doing a workaround to prevent this.

We copy all files which we want to be included in the npm module to the dist folder. Currently, this is the README.md, LICENSE and the package.json. We later need to publish from the dist folder.

You can try this out by running yarn build; cd dist; yarn pack (yarn pack is like yarn publish, except that it doesn't publish the created module). Then inspect the contents of the generated .tar.gz file which represent the module that would be published on npm.

Copying of package.json to dist and publishing from there lets consumers avoid the dist in imports as the folder is no longer part of the module.

package.json getting copied to dist is also why the main field in package.js contains ui-kit.cjs.js instead of dist/ui-kit.cjs.js.

This preparation should make it fairly easy to set up the release script in #2.

`main` and `module`

The package.json has two fields, "main": "ui-kit.cjs.js" and "module": "ui-kit.esm.js". In case consumers of ui-kit use a bundler which supports the module field, then the consumer will benefit from treeshaking when building. This means that only the used components will end up in the final application build.

Consumers using the main field will load the whole UI Kit which is quite big. This is not recommended.

Uglify

The UI Kit is served as-is. Uglifying is the responsibility of the consumer of the UI Kit.

CSS

We are not creating an external CSS file at the moment. All CSS (global and modular) required to run UI-Kit is packed into the index.esm.js file. Unfortunately the CSS does not benefit from treeshaking yet. So even when a component is not being used, its styles will still get included into the parent, and injected into the document head.

See egoist/rollup-plugin-postcss#98 for more.

We can try to tackle this separately later. I have a PoC where treeshaking CSS is possible.

SVG (and other assets)

We convert the Icons to React Components while exposing regular SVG and PNG images from materials as plain assets.

Messages

We export the messages from index.js as en, de and es for now.
They can also be used through import { en, de, es } from 'ui-kit/i18n/messages'.

This setup is not final and serves mostly to avoid breaking changes there for now.

proxy exports copying

Some consumers "reach into" ui-kit to import files like this:

import PrimaryButton from 'ui-kit/buttons/primary-button'

This is deprecated and should not be used anymore.

We want to avoid breaking changes for now, thus we have to keep supporting consumers which reach into UI-Kit. We do this by recreating the previous folder structure by copying proxy-exports into the final dist folder. The copied files merely re-export from the generated index.esm.js files.

We will remove this once the consumers have been updated.

circular dependency warnings after build

Right now we attempt to bundle react-virtualized and styled-components into the main bundle. These both result in some circular dependency warnings when bundling. We plan to get rid of both of these libraries soon, so we did not spend time to attempt to fix this.

`thread-loader`

I did some experimentation and it turns out that the build time is 20s faster on the initial build, and 4s faster on following builds (with filled cache) when not using thread-loader, so I removed it. This is an unrelated change and concerns the Webpack setup only.

Todo

adapt netlify to execute the right command

Possible Future Improvements:

verify treeshaking works (CSS doesn't work for sure)
enable treeshaking of CSS
move Components (Contraints, Spacings) out of materials

💐And finally, a thanks a ton to @montezume for setting the branch up in the first place, and for pairing on this with me!

This is what the final bundle published to npm would look like: ui-kit.zip

Closes #7

montezume · 2018-09-07T14:44:38Z

.storybook/webpack.config.js

-    // For normal svg files (not icons) we should load the file normally
-    // and simply use it as a `<img src/>`.
-    {
-      test: function testForNormalSvgFiles(fileName) {


we weren't actually bundling any "normal" SVG files, so removed this for readability.

Nitpick: can we keep it there in case we need it in the future? I don't mind though to remove it...

If you want I can re-add this, what I like about removing it is that it will make really apparent if someone adds SVG and PNG's because they will need to modify the webpack config

Sure, I don't mind. We can keep it removed for now

montezume · 2018-09-07T14:45:01Z

.storybook/webpack.config.js

-    // smaller than specified limit in bytes as data URLs to avoid requests.
-    // A missing `test` is equivalent to a match.
-    {
-      test: /\.png$/,


again unused, so removing to make the config easier to read.

montezume · 2018-09-07T14:45:52Z

package.json

@@ -89,11 +93,13 @@
    "moment-timezone": "0.5.21",
    "omit-empty": "0.4.1",
    "postcss": "7.0.2",
-    "postcss-color-mod-function": "2.4.3",
-    "postcss-import": "12.0.0",
+    "postcss-color-mod-function": "^2.4.3",


ah maybe we should change these and remove the ^

montezume · 2018-09-07T14:49:24Z

rollup.config.js

+      }),
+      json(),
+      svg({
+        include: ['**/*.svg'],


ah this can be removed

montezume · 2018-09-07T14:49:32Z

rollup.config.js

+      // We tried all copy plugins of rollup, and couldn't get a single one to
+      // do this. So, the shell it is!
+      execute(
+        'cp -R package.json README.md LICENSE materials proxy-exports/ dist/'


Woops, had forgotten to copy the materials/images

emmenko

🤩🤩🤩

Fuck**g yeah! 👩‍🎤🎸

emmenko · 2018-09-07T15:30:52Z

.storybook/webpack.config.js

-    // For normal svg files (not icons) we should load the file normally
-    // and simply use it as a `<img src/>`.
-    {
-      test: function testForNormalSvgFiles(fileName) {


Nitpick: can we keep it there in case we need it in the future? I don't mind though to remove it...

i18n/index.js

emmenko · 2018-09-07T15:31:48Z

package.json

@@ -7,15 +7,18 @@
  "publishConfig": {
    "access": "public"
  },
-  "main": "src/index.js",
+  "main": "ui-kit.cjs.js",
+  "module": "ui-kit.esm.js",


In node land ESM modules (if my memory is still correct) can only be supported by the mjs extension. Should we go for the same extension here?

Ah, I also still wanted to look into the difference of specifying es and esm as the output.

emmenko · 2018-09-07T15:32:20Z

package.json

-    "build": "build-storybook -o .public",
-    "start": "start-storybook -p 9002",
+    "prestorybook:start": "node ./scripts/generate-base-colors.js && node ./scripts/generate-base-shadows.js && node ./scripts/generate-colors-for-story.js",
+    "storybook:build": "build-storybook -o .public",


Cool, I'll push the netlify config file so we can change the build script from there.

emmenko · 2018-09-07T15:32:54Z

package.json

-    "start": "start-storybook -p 9002",
+    "prestorybook:start": "node ./scripts/generate-base-colors.js && node ./scripts/generate-base-shadows.js && node ./scripts/generate-colors-for-story.js",
+    "storybook:build": "build-storybook -o .public",
+    "build": "NODE_ENV=production rollup -c",


Niptick: I usually see cross-env being used. Should we add it as well?

emmenko · 2018-09-07T15:33:26Z

proxy-exports/buttons/accessible-button/index.js

@@ -0,0 +1,2 @@
+// NOTE: this is a proxy export for backwards compatibility
+export { AccessibleButton as default } from '../../ui-kit.esm';


emmenko · 2018-09-07T15:34:18Z

proxy-exports/icons/index.js

@@ -0,0 +1,93 @@
+export { AddBoldIcon } from '../ui-kit.esm';


Is this list generated or not?

We created it manually in hope of all future consumers importing from ui-kit.esm instead.

We hope to be able to remove all these proxy-exports soon. So it didn't feel reasonable to write a script for it if we're going to remove it soon anyways. We want any new icons to be consumed as a named export from the main file.

Sure, I also improved the docs a bit to mention this: 4985d0b

rollup.config.js

emmenko · 2018-09-07T15:37:15Z

rollup.config.js

+      }),
+      // this is used for importing both vendor css (from node_modules) and for
+      // importing our global css which uses tokens that require postcss plugins
+      // (such as color-mod) to be compiled properly.


emmenko · 2018-09-07T15:37:57Z

rollup.config.js

+      }),
+      // copy static files over
+      // We tried all copy plugins of rollup, and couldn't get a single one to
+      // do this. So, the shell it is!


emmenko · 2018-09-07T16:04:02Z

Build is passing now! 🚀

emmenko · 2018-09-07T16:38:05Z

Just had a look at the current main export 😅

Should we maybe group e.g. the icons? import { Icons } from '...'; Icons.AddIcon

export {
  AccessibleButton,
  FlatButton$1 as FlatButton,
  GhostButton,
  IconButton$1 as IconButton,
  LinkButton,
  PrimaryButton,
  secondaryButton as SecondaryButton,
  secondaryIconButton as SecondaryIconButton,
  Collapsible,
  CollapsibleMotion,
  PrimaryActionDropdown,
  Option as PrimaryActionDropdownOption,
  FieldLabel,
  TextField,
  Wrapped as AsyncCreatableSelectInput,
  Wrapped$1 as AsyncSelectInput,
  Enhanced as CreatableSelectInput,
  dateInput as DateInput,
  dateTimeInput as DateTimeInput,
  Wrapped$2 as LocalizedMultilineTextInput,
  LocalizedTextInput,
  MoneyInput,
  Enhanced$1 as MultilineTextInput,
  NumberInput,
  PasswordInput,
  Enhanced$2 as SelectInput,
  TextInput,
  TimeInput$1 as TimeInput,
  Label,
  LoadingSpinner,
  ErrorMessage,
  CollapsiblePanel,
  Label$1 as Stamp,
  Constraints,
  Spacings,
  Table$1 as Table,
  BaseTable,
  Cell as TableCell,
  Tag,
  TimeRangePicker,
  Text,
  withMouseDownState,
  withMouseOverState,
  AddBoldIcon,
  AddIcon,
  AddressBillingIcon,
  AddressShippingIcon,
  AngleDownIcon,
  AngleLeftIcon,
  AngleRightIcon,
  AngleUpIcon,
  ArrowGraphDownIcon,
  ArrowGraphLeftIcon,
  ArrowGraphRightIcon,
  ArrowGraphUpIcon,
  ArrowLeftIcon,
  ArrowLongDownIcon,
  ArrowRightIcon,
  ArrowTriangleDownIcon,
  ArrowTriangleUpIcon,
  ArrowsIcon,
  BackIcon,
  BoxProductIcon,
  BoxIcon,
  BrainIcon,
  CalendarIcon,
  CameraIcon,
  CaretDownIcon,
  CaretUpIcon,
  CartIcon,
  CategoryTreeIcon,
  ChainBrokenIcon,
  ChainIcon,
  CheckActiveIcon,
  CheckInactiveIcon,
  ClipboardIcon,
  ClockIcon,
  CloseBoldIcon,
  CloseIcon,
  CodeViewIcon,
  CoinsIcon,
  ColumnsIcon,
  CopyIcon,
  CubesIcon,
  CustomSettingsIcon,
  CustomViewIcon,
  CustomerFilledIcon,
  CustomerIcon,
  DeleteFilledIcon,
  DeleteIcon,
  DoneIcon,
  DotIcon,
  DownloadIcon,
  DragDropIcon,
  DragIcon,
  EditIcon,
  ErrorIcon,
  ExpandIcon,
  ExportIcon,
  FilterIcon,
  FlagFullfiledIcon,
  FlagIcon,
  FlameIcon,
  GraphIcon,
  GridIcon,
  ImportIcon,
  InfoIcon,
  InformationIcon,
  ListIcon,
  LogoutIcon,
  MailIcon,
  MinimizeIcon,
  NestedViewIcon,
  PagesIcon,
  PinActiveIcon,
  PinIcon,
  ProjectSettingsIcon,
  RefreshIcon,
  RestoreIcon,
  ReturnInfoIcon,
  RevertIcon,
  ReviewIcon,
  SearchIcon,
  SortingIcon,
  SpeedometerIcon,
  SplitIcon,
  SuccessIcon,
  SupportIcon,
  SwitcherIcon,
  TableIcon,
  TagDiscountIcon,
  TagMultiIcon,
  TagIcon,
  VerifiedIcon,
  WarningIcon,
  WorldIcon,
  en$1 as en,
  de$3 as de,
  es$1 as es,
  index$d as Radio,
  Checkbox,
  toggle as Toggle,
};

dferber90 · 2018-09-07T16:39:58Z

Sure :) The way we export languages is also pretty bad right now. We are exporting them as import { en, de, es } from 'ui-kit'. We should probably group them as well.

emmenko · 2018-09-07T16:40:39Z

package.json

-    "format:css": "prettier --parser postcss --write '{src}/**/*.css'",
-    "format:md": "prettier --parser markdown --write '{.github,src,examples}/**/*.md'",
+    "format:js": "prettier --write '{.storybook,examples,scripts,src}/**/*.js' *.js",
+    "format:css": "prettier --write --parser css '{.storybook,examples,src}/**/*.css'",


PS: apparently postcss has been deprecated, it's only css now.

emmenko · 2018-09-07T16:52:50Z

Oh, the css module proxy exports need to be fixed

Module build failed (from /xxx/node_modules/postcss-loader/src/index.js):
Error: Failed to find '../../src/materials/colors/base-colors.mod.css'
  in [
    /xxx/node_modules/@commercetools-frontend/ui-kit/materials/colors
  ]

emmenko · 2018-09-07T16:53:47Z

We should probably copy the original ones into the dist/materials folder

montezume · 2018-09-10T09:31:08Z

rollup.config.js

@@ -101,12 +101,14 @@ export default [
          ],
        },
      }),
+      // copy materials css over. don't even ask :(


montezume · 2018-09-10T09:51:42Z

@emmenko the mod.css exports should pass the MC build now

emmenko · 2018-09-10T09:58:52Z

I'll give it another try, thanks

emmenko · 2018-09-10T10:12:10Z

Tested it locally. Works now!! 🙌

Add cjs module export, rearrange npm scripts, fix for namespace exports, add cleanup rollup plugin, and copy files and folders to dist.

…onfig

…nv setting

montezume · 2018-09-10T12:03:34Z

@dferber90 @emmenko ready when you are 🚀

emmenko · 2018-09-10T12:17:25Z

This is now published to npm under the tag rollup-bundle (or version 1.0.0-rollup-bundle)

yarn add @commercetools-frontend/ui-kit@rollup-bundle

emmenko · 2018-09-10T12:18:07Z

There is one small issue with the package.json while releasing...I'll try to push a fix

montezume · 2018-09-10T13:36:08Z

scripts/bundle-copy.js

@@ -0,0 +1,37 @@
+// NOTE: this script is only necessary as long as we need to support
+// proxy exports for backwards compatibility.


Unless we plan on including the png / svg images in materials in the final bundle, we will need to keep this, right?

Yeah, but then I would put it back to the rollup config once we have settled on our final structure.

montezume · 2018-09-10T13:39:15Z

scripts/bundle-copy.js

+);
+// 3. Copy only specific fields of the `package.json`
+// This is to avoid problems on executing pre/post scripts
+// while publishing from inside the `dist` folder.


dferber90 added 🙏Status: Code Review 🔮 Type: Chore labels Sep 7, 2018

dferber90 assigned dferber90 and montezume Sep 7, 2018

dferber90 requested review from emmenko and tdeekens September 7, 2018 14:39

montezume reviewed Sep 7, 2018

View reviewed changes

emmenko reviewed Sep 7, 2018

View reviewed changes

montezume reviewed Sep 10, 2018

View reviewed changes

montezume and others added 12 commits September 10, 2018 13:48

feat: first-attempt at adding rollup

36b8a22

feat(build-process): fix css bundling

514efaf

feat(build-process): fix svg bundling ^_^

9447097

chore(build-process): dep cleanup"

3948926

chore(build-process): fixup rollup config

f78fae6

feat(build-process): add messages export

1277ed5

chore: moved proxy exports

77dc340

chore(build-process): add cjs module and script fixups

8fbfc62

Add cjs module export, rearrange npm scripts, fix for namespace exports, add cleanup rollup plugin, and copy files and folders to dist.

chore(build-process): remove thread loader from webpack config

8231091

chore: copy files from rollup

8ab4854

chore: inline plugins in rollup config

7f2ea85

chore: cp -r -> -R

d9a7d09

dferber90 and others added 13 commits September 10, 2018 13:48

chore: rename demo to storybook in scripts

c55c554

chore(build-process): remove remnants of thread-loader from webpack.c…

449ee4d

…onfig

chore(build-process): remove unused webpack loaders

0f014e0

chore(build-process): fixup deps, remove unused rollup deps

98ae834

chore: disable eslint for proxy-exports dir

a9c74e6

chore(build-systems): add cross-env for node-env setting

12e5305

chore: add netlify.toml config

52f9d04

docs(readme): improve imports docs

df4d6c7

chore: run prettier, adjust glob matches

924d231

chore(build-process): add rollup replace plugin for production node e…

8e67d76

…nv setting

chore(build-process): add srync copy for orig materials

b472207

chore(build-process): remove unused proxy exports

1e8b373

chore(build-process): copy images as well

c1ff955

montezume force-pushed the ml-add-rollups branch from cda7593 to c1ff955 Compare September 10, 2018 11:49

chore(process-build): add table-cell proxy export

a85dd3d

chore: add script for copying files to dist folder

99ed786

montezume reviewed Sep 10, 2018

View reviewed changes

montezume merged commit 26f3df2 into master Sep 10, 2018

montezume deleted the ml-add-rollups branch September 10, 2018 14:30

		@@ -0,0 +1,2 @@
		// NOTE: this is a proxy export for backwards compatibility
		export { AccessibleButton as default } from '../../ui-kit.esm';

		@@ -0,0 +1,93 @@
		export { AddBoldIcon } from '../ui-kit.esm';

		@@ -0,0 +1,37 @@
		// NOTE: this script is only necessary as long as we need to support
		// proxy exports for backwards compatibility.

Add bundling (rollup) #34

Add bundling (rollup) #34

Conversation

dferber90 commented Sep 7, 2018 • edited Loading

Usage

Consumers

Publishing

main and module

Uglify

CSS

SVG (and other assets)

Messages

proxy exports copying

circular dependency warnings after build

thread-loader

Todo

Possible Future Improvements:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

montezume Sep 10, 2018 • edited Loading

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

emmenko 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

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

Choose a reason for hiding this comment

emmenko commented Sep 7, 2018

emmenko commented Sep 7, 2018

dferber90 commented Sep 7, 2018

Choose a reason for hiding this comment

emmenko commented Sep 7, 2018

emmenko commented Sep 7, 2018

Choose a reason for hiding this comment

montezume commented Sep 10, 2018

emmenko commented Sep 10, 2018

emmenko commented Sep 10, 2018

montezume commented Sep 10, 2018

emmenko commented Sep 10, 2018

emmenko commented Sep 10, 2018

Choose a reason for hiding this comment

emmenko Sep 10, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dferber90 commented Sep 7, 2018 •

edited

Loading

`main` and `module`

`thread-loader`

montezume Sep 10, 2018 •

edited

Loading

emmenko Sep 10, 2018 •

edited

Loading