feat: split codebase into packages and set up monorepository #1177

montezume · 2019-11-13T12:05:25Z

No description provided.

tdeekens · 2019-11-13T12:07:11Z

I like this approach of exporting a combined package. Maybe something the MC will always use or teams decide to only pick what they want.

I think generally exporting multiple bundles and modules is nice and the approach to gradually migrate there is very nice.

Have you checked for our MC apps what porportion of the bundle size is uikit so we can evaluate the gains for us?

tdeekens · 2019-11-13T12:08:32Z

packages/multiline-text-input/package.json

@@ -0,0 +1,28 @@
+{
+  "name": "@ui-kit/multiline-text-input",


Nit but I bet this namespace will be taken. Also nested namespaces don't work. So @ct-fe/uikit/text-input won't do. I guess @ct-fe/uikit-textinput could be something.

yeah good point.

@commercetools-frontend/ui-kit-text-input should work?

Yes, I think so. Or a completley new scope #commercetools-uikit/text-input.

montezume · 2019-11-13T12:36:04Z

@tdeekens I'll do some research on the FE side, good call.

montezume · 2019-11-13T12:43:11Z

@tdeekens with application-orders, ui-kit or ui-kit related dependencies (react-select, immutable js, slate, etc etc) make up (all gzipped numbers) seem to make up about 1/2 of the bundle size.

tdeekens · 2019-11-13T12:48:46Z

Oh wow. Immutable.js? Gotta do yarn why on that one I guess.

montezume · 2019-11-13T12:49:07Z

@tdeekens it's from the rich-text-input...

emmenko

The approach looks promising. I think we need to plan a bit how we want to:

move to a monorepo setup as the first step
split up the source code into their respective packages in the long term

I would expect eventually to have the source code in each package, then the original package @ct-fe/ui-kit is just a mere proxy from all other packages (no source code there).

emmenko · 2019-11-13T15:32:36Z

package.json

@@ -86,7 +89,8 @@
    "slate-react": "0.22.10",
    "tiny-invariant": "1.0.6",
    "use-popper": "1.1.6",
-    "warning": "4.0.3"
+    "warning": "4.0.3",


In a monorepo, the root package.json should not have any runtime dependencies, as it simply does not make sense to. Instead runtime deps should be defined by each package.

yeah I hadn't got to that party yet 😄

emmenko · 2019-11-13T15:38:47Z

packages/avatar/package.json

+  "author": "",
+  "license": "MIT",
+  "dependencies": {
+    "@emotion/core": "10.0.22",


I'm wondering "how easy" it would be here to miss some runtime dep.

So far it seems like you get warnings in the build but not failures.

I mean within the monorepo it shouldn't matter, as packages get hoisted. I'm talking about consumers of the package. If some runtime dep is not defined, you will probably get an error then.

I guess this would make more sense to do once the source code is moved into its own package.

emmenko

What is the plan with this PR? Should we move everything at once? Is it just for testing things out and then we start migrating things one by one in separate PRs?

If we start moving components into their own package, I would prefer then to do it "properly" by moving the source code and have the dependency + export in the main ui-kit package.

emmenko · 2019-11-14T08:17:23Z

package.json

+    "packages/*",
+    "packages/buttons/*",
+    "packages/typography/*",
+    "packages/inputs/*"


Should we keep the packages flat?

emmenko · 2019-11-14T08:19:17Z

packages/avatar/package.json

+  "author": "",
+  "license": "MIT",
+  "dependencies": {
+    "@emotion/core": "10.0.22",


I mean within the monorepo it shouldn't matter, as packages get hoisted. I'm talking about consumers of the package. If some runtime dep is not defined, you will probably get an error then.

I guess this would make more sense to do once the source code is moved into its own package.

emmenko · 2019-11-14T08:20:09Z

rollup.config.js

@@ -26,7 +32,7 @@ const configureRollupPlugins = (options = {}) =>
    // See also https://medium.com/@kelin2025/so-you-wanna-use-es6-modules-714f48b3a953
    // Transpile sources using our custom babel preset.
    babel({
-      exclude: ['node_modules/**'],
+      exclude: path.join(__dirname, '/node_modules/**'),


Much better now 😉

emmenko · 2019-11-14T08:20:59Z

rollup.config.js

-      file: pkg.module,
-      format: 'esm',
+const createConfig = cliArgs => {
+  return [


Nit: unnecessary return block

emmenko · 2019-11-14T08:21:23Z

src/components/avatar/avatar.js

@@ -1,8 +1,8 @@
-import React from 'react';
+/** @jsx jsx */


What's this for? 🤔

because the only JSX rendered by this component using the css prop, the built version drops all usages of react.createELement and replaces it with JSX

so we get warnings about unused 'react' import

Here's an example of what Text looks like

import 'react'; import PropTypes from 'prop-types'; import { FormattedMessage } from 'react-intl'; import requiredIf from 'react-required-if'; import { isNil } from 'lodash-es'; import { oneLine } from 'common-tags'; import { css, jsx } from '@emotion/core'; import vars from '@commercetools-uikit/custom-properties';

... more stuff

var Text = function Text(_ref) { var intlMessage = _ref.intlMessage, children = _ref.children; return intlMessage ? jsx(FormattedMessage, intlMessage) : children; };

can remove this

emmenko · 2019-11-14T08:22:40Z

src/components/buttons/flat-button/flat-button.js

@@ -2,9 +2,9 @@ import React from 'react';
 import PropTypes from 'prop-types';
 import { css } from '@emotion/core';
 import { omit } from 'lodash';
-import vars from '../../../../materials/custom-properties';
+import Text from '@commercetools-uikit/text';


Wondering if it would make sense to have the package named @commercetools-uikit/typography, which exports individual components like import { Header, Body, ... }

emmenko · 2019-11-14T08:23:35Z

src/components/icons/create-styled-icon.js

@@ -1,7 +1,7 @@
 import PropTypes from 'prop-types';
 import invariant from 'tiny-invariant';
 import styled from '@emotion/styled';
-import vars from '../../../materials/custom-properties';
+import vars from '@commercetools-uikit/custom-properties';


Should we name this package something like @commercetools-uikit/design-system or @commercetools-uikit/design-tokens? 🤔

emmenko · 2019-11-14T08:24:39Z

src/components/inputs/multiline-text-input/multiline-text-input.js

@@ -3,9 +3,9 @@ import PropTypes from 'prop-types';
 import requiredIf from 'react-required-if';
 import { useIntl } from 'react-intl';
 import { css } from '@emotion/core';
+import { AngleUpIcon, AngleDownIcon } from '@commercetools-uikit/icons';


As far as I remember, one of the issues with tree shaking was related to the svgs...should we maybe publish each icon individually? If we bundle them all together we might end up with the same problem 🤔

yeah, that's what I wanted to mess around with now. 👍 Going to investigate in the VRT app how treeshaking works with the icons.

Ah yeah, so right now if you use MultilineTextInput, you end up with all of the icons. I'll look into alternatives now.

emmenko · 2019-11-14T08:25:09Z

src/components/table/base-table/base-table.js

@@ -2,7 +2,7 @@ import React from 'react';
 import PropTypes from 'prop-types';
 import { CellMeasurer, CellMeasurerCache, MultiGrid } from 'react-virtualized';
 import sortBy from 'lodash/sortBy';
-import getScrollbarSize from 'dom-helpers/scrollbarSize';
+import getScrollbarSize from 'dom-helpers/util/scrollbarSize';


Was this a bug fix? 🤔

I'm confused about this. There must be some weird resolution happening, because when I open node_modules on master, the file is at scrollbarSide, yet when I open node modules here, it's at utils/scrollbarSize and you get a lint warning 🤔

emmenko · 2019-11-14T08:25:38Z

visual-testing-app/webpack.config.js

+      // For svg icons, we want to get them transformed into React components
+      // when we import them.
+      {
+        test: /\.react\.svg$/,


Can you clarify why we need this now?

we don't, can remove it

Is this still valid?

ah yeah, should not noeed that sorry

tdeekens · 2019-12-03T18:10:46Z

bundlesize.config.json

@@ -2,7 +2,7 @@
  "files": [
    {
      "path": "visual-testing-app/dist/ui-kit.js",
-      "maxSize": "130 kB"
+      "maxSize": "150 kB"


Wasn't the point of this to... just kidding 😄.

…rds compatibility

emmenko · 2019-12-04T08:54:09Z

This is ready to go now from my side. We can start moving code into their packages as follow ups.

jonnybel

Everything seems OK. Great work!

emmenko · 2019-12-04T10:43:29Z

Great, I’ll merge this later today then.

montezume force-pushed the ml-mono branch from a9ca0c0 to 6715a9f Compare November 13, 2019 12:06

tdeekens reviewed Nov 13, 2019

View reviewed changes

emmenko reviewed Nov 13, 2019

View reviewed changes

montezume force-pushed the ml-mono branch from 5229d2c to ac4fada Compare November 13, 2019 16:08

emmenko reviewed Nov 14, 2019

View reviewed changes

montezume force-pushed the ml-mono branch 2 times, most recently from c31f6e7 to 2ec5b6b Compare November 19, 2019 14:38

montezume force-pushed the ml-mono branch 2 times, most recently from 76a28ab to a320e76 Compare November 26, 2019 09:52

montezume added 12 commits November 27, 2019 12:19

chore: merge

7de3275

refactor: try mono repo

f317b62

chore: more packages

440976c

chore: start creating presets

b927192

chore: finish buttons

15317c0

chore: add some more packages

fff1b82

chore: fixup

57178d7

chore: date input stuff

566ebfc

chore: more inputs

a387e34

chore: more and more

5157ddb

chore: more components

ee09468

chore: more and more fixup

539d976

montezume force-pushed the ml-mono branch from d2735a8 to 539d976 Compare November 27, 2019 11:19

montezume added 3 commits November 27, 2019 12:28

chore: add another field

9984975

chore: more fields

8007ee0

chore: more

3ce0d22

emmenko added 4 commits December 3, 2019 17:00

refactor: use script to inject version on each package

6daf71b

refactor: bump bundlesize limit for uikit

450e720

refactor: add inputs preset

d51a514

refactor: drop input utils package

acaf1d2

emmenko force-pushed the ml-mono branch from 9319a25 to acaf1d2 Compare December 3, 2019 16:37

emmenko added 4 commits December 3, 2019 17:44

refactor: add fields preset

53004db

fix: exec file permissions

941347b

fix: missing exports

25c9b85

fix: do not use version in preset packages

cb4274f

emmenko force-pushed the ml-mono branch from 8db81d9 to cb4274f Compare December 3, 2019 17:20

chore: bump bundlesize

02b91d5

emmenko changed the title ~~WIP - migration to monorepo~~ feat: split codebase into packages and set up monorepository Dec 3, 2019

emmenko added Semver: BREAKING 💥 Type: Breaking Change 🚀 Type: New Feature Something new labels Dec 3, 2019

tdeekens reviewed Dec 3, 2019

View reviewed changes

emmenko removed the Semver: BREAKING label Dec 3, 2019

emmenko added 2 commits December 3, 2019 19:14

chore: update github labels

699ac46

feat: setup lerna changelog

5867076

emmenko approved these changes Dec 4, 2019

View reviewed changes

refactor: keep materials and i18n folders in ui-kit preset for backwa…

1f8a0bd

…rds compatibility

emmenko added 2 commits December 4, 2019 10:10

fix: missing files for npm publish

698726e

chore: revert alpha version

f60e43f

jonnybel approved these changes Dec 4, 2019

View reviewed changes

emmenko removed the 💥 Type: Breaking Change label Dec 4, 2019

tdeekens approved these changes Dec 4, 2019

View reviewed changes

emmenko merged commit 4ed4f60 into master Dec 4, 2019

emmenko deleted the ml-mono branch December 4, 2019 16:04

feat: split codebase into packages and set up monorepository #1177

feat: split codebase into packages and set up monorepository #1177

Conversation

montezume commented Nov 13, 2019 • edited by emmenko Loading

tdeekens commented Nov 13, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

montezume commented Nov 13, 2019

montezume commented Nov 13, 2019 • edited Loading

tdeekens commented Nov 13, 2019

montezume commented Nov 13, 2019

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

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

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 Dec 4, 2019

jonnybel left a comment

Choose a reason for hiding this comment

emmenko commented Dec 4, 2019

montezume commented Nov 13, 2019 •

edited by emmenko

Loading

montezume commented Nov 13, 2019 •

edited

Loading