chore(mixed): fix imports and introduce bundler test #570

layershifter · 2018-12-06T13:17:49Z

This PR:

adds a test for Rollup bundler
cleanups imports to avoid circular issues
fixes Impossible to build project with Parcel or Rollup #547

I also tested a build with Parcel manually, it works like a charm ⭐️

build/gulp/tasks/test-projects.tsx

layershifter · 2018-12-06T13:19:39Z

types/classnames/index.d.ts

+  const classNames: ClassNamesFn
+
+  export default classNames
+}


We decided to use custom typings for this module.

src/lib/colorUtils.ts

src/components/Avatar/Avatar.tsx

build/gulp/tasks/test-bundlers.tsx

…ore/fix-imports

package.json

…ore/fix-imports # Conflicts: # CHANGELOG.md

…/stardust-ui/react into chore/fix-imports # Conflicts: # CHANGELOG.md

CHANGELOG.md

miroslavstastny · 2018-12-10T12:33:49Z

build/gulp/scaffold/rollup.config.js

+import resolve from 'rollup-plugin-node-resolve'
+import commonjs from 'rollup-plugin-commonjs'
+
+const lodashExports = [


(!) Missing exports https://rollupjs.org/guide/en#error-name-is-not-exported-by-module- node_modules/@stardust-ui/react/dist/es/components/Dropdown/Dropdown.js pickBy is not exported by node_modules/lodash/lodash.js

How is this going to scale? Whenever a new function is used, do I need to remember I have to add it here?

Yes, it's a real problem 👎 lodash is CJS module, so the there is only one option.
If we will use Babel and babel-plugin-lodash this problem will gone.

Resolved with external and output.globals, now is much cleaner

build/gulp/tasks/test-projects.tsx

miroslavstastny · 2018-12-10T12:37:28Z

build/gulp/tasks/test-bundlers.tsx

+  logger(`✔️Source and bundler's config were created`)
+
+  await runIn(tmpDirectory)(`yarn rollup -c`)
+  logger(`✔️Example project was successfully built: ${tmpDirectory}`)


It seems we are ignoring rollup warnings. Are we ok with it or would we like to use this as an opportunity to tighten our rules and require warning-free build in rollup? (no warnings are currently allowed in webpack build)

I reworked it, now we have a warning whitelist, other warnings will throw.

We can fix all left circular imports in a separate PR because it will require more work.

kuzhelov · 2018-12-10T12:39:30Z

.circleci/config.yml

@@ -58,6 +58,9 @@ jobs:
 #      - run:


when we are planning to return this? And, actually, what was specific reason to disable these tests - currently we have no any checks that any client is able to consume our library

After #550 will be merged

kuzhelov · 2018-12-10T13:19:35Z

build/gulp/scaffold/rollup.config.js

@@ -0,0 +1,110 @@
+import replace from 'rollup-plugin-replace'


this should be placed somewhere under test-projects at the very least (we should create corresponding dir) - so that this client-oriented tests functionality will be cohesively structured

It's already placed in the separate dir. I don't want to be so specific for now.

yes, but the same reasoning would apply well in case it will be put in any dir - thus, I am not sure that this is a valid argument.

The reasoning behind my ask is the following: currently, with the dir introduced at the place where you are suggesting, we would have the following project structure:

/build/gulp /scaffold /tasks /test-projects

So, reasonable question is why scaffold directory is placed at this level of folder hierarchy? Is its content shared by any agents/tasks? The answer is "no" - the only place where it is used is test-projects module. Thus, I would suggest to put these bits to the module those belong to at the moment.

Is its content shared by any agents/tasks? The answer is "no" - the only place where it is used is test-projects module.

At now, yes. But in the future, I think that we will have other tasks that will should also have files and templates.

I must agree here that better directory structure would reflect exactly what we test even. Having structure, something like:

/build/gulp /tasks /test-projects /cra-ts /parcel ...

will be much more cleaner, and it will be obvious what scenarios we got covered so far...

@kuzhelov @mnajdova you convinced me and I made these changes 👍

kuzhelov · 2018-12-10T13:21:33Z

.circleci/config.yml

@@ -58,6 +58,9 @@ jobs:
 #      - run:
 #          name: Project Tests
 #          command: yarn test:projects:cra-ts
+      - run:
+          name: Bundler Tests
+          command: yarn test:bundlers:rollup


would suggest test:projects:rollup - as this won't introduce additional entropy and will still suggest that stack that client is using for her project

Moved into single entry, yarn test:projects 👍

…ore/fix-imports # Conflicts: # CHANGELOG.md

kuzhelov · 2018-12-10T23:00:14Z

.circleci/config.yml

-#          command: yarn test:projects:cra-ts
+      - run:
+          name: Project Tests
+          command: yarn test:projects


kuzhelov · 2018-12-10T23:01:48Z

build/gulp/tasks/test-projects.tsx

+
+task(
+  'test:projects',
+  series(


actually, can we run them in parallel? It seems that there is no shared state between these tasks

I will left it as is for now. When this task will be enabled, we will able to test that it works in parallel. However, I almost sure that it will work.

…ore/fix-imports

chore(mixed): fix imports and introduce bundler test

07fa488

layershifter requested review from dvdzkwsk, kuzhelov, levithomason, miroslavstastny and mnajdova as code owners December 6, 2018 13:17

layershifter added the 🚧 WIP label Dec 6, 2018

layershifter commented Dec 6, 2018

View reviewed changes

build/gulp/tasks/test-projects.tsx Show resolved Hide resolved

layershifter commented Dec 6, 2018

View reviewed changes

src/lib/colorUtils.ts Outdated Show resolved Hide resolved

layershifter commented Dec 6, 2018

View reviewed changes

src/components/Avatar/Avatar.tsx Show resolved Hide resolved

layershifter commented Dec 6, 2018

View reviewed changes

build/gulp/tasks/test-bundlers.tsx Outdated Show resolved Hide resolved

layershifter added 5 commits December 7, 2018 10:50

revert import change

0d58036

do not build dll on build:dist

2b7a4dd

rewrite bundler task

1929f00

restore imports, introduce transformer

a9683e2

fix CI command

363d3b4

layershifter force-pushed the chore/fix-imports branch from f294915 to 363d3b4 Compare December 7, 2018 10:34

layershifter added 2 commits December 7, 2018 12:43

Merge branch 'master' of https://github.com/stardust-ui/react into ch…

aa8ed3b

…ore/fix-imports

restore command

8b8a748

layershifter commented Dec 7, 2018

View reviewed changes

package.json Show resolved Hide resolved

layershifter commented Dec 7, 2018

View reviewed changes

package.json Show resolved Hide resolved

layershifter changed the title ~~[WIP] chore(mixed): fix imports and introduce bundler test~~ chore(mixed): fix imports and introduce bundler test Dec 7, 2018

layershifter added 🚀 ready for review 🧰 fix Introduces fix for broken behavior. and removed 🚧 WIP labels Dec 7, 2018

layershifter added 2 commits December 7, 2018 13:20

add entry

7b9987d

Merge branch 'master' of https://github.com/stardust-ui/react into ch…

d567968

…ore/fix-imports # Conflicts: # CHANGELOG.md

layershifter mentioned this pull request Dec 10, 2018

refactor(lib): remove createColorVariants and setColorLightness utils #583

Merged

miroslavstastny added the 🚧 WIP label Dec 10, 2018

miroslavstastny removed the 🚀 ready for review label Dec 10, 2018

layershifter added 3 commits December 10, 2018 12:42

Merge branches 'chore/fix-imports' and 'master' of https://github.com…

abbf734

…/stardust-ui/react into chore/fix-imports # Conflicts: # CHANGELOG.md

remove transformer

34c32b0

add typings, fix import

a7deb19

layershifter added 🚀 ready for review and removed 🚧 WIP labels Dec 10, 2018

miroslavstastny reviewed Dec 10, 2018

View reviewed changes

kuzhelov reviewed Dec 10, 2018

View reviewed changes

fix typo

da99b8d

kuzhelov reviewed Dec 10, 2018

View reviewed changes

layershifter added 2 commits December 10, 2018 15:41

review improvements

a2cef68

Merge branch 'master' of https://github.com/stardust-ui/react into ch…

a105f7e

…ore/fix-imports # Conflicts: # CHANGELOG.md

kuzhelov reviewed Dec 10, 2018

View reviewed changes

.circleci/config.yml

# command: yarn test:projects:cra-ts

- run:

name: Project Tests

command: yarn test:projects

Copy link

Contributor

kuzhelov Dec 10, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

kuzhelov reviewed Dec 10, 2018

View reviewed changes

layershifter added 2 commits December 11, 2018 12:35

update directory structure

09d2dd6

Merge branch 'master' of https://github.com/stardust-ui/react into ch…

38770f4

…ore/fix-imports

kuzhelov approved these changes Dec 11, 2018

View reviewed changes

kuzhelov added ready for merge and removed 🚀 ready for review labels Dec 11, 2018

layershifter merged commit 6eff029 into master Dec 11, 2018

layershifter deleted the chore/fix-imports branch December 11, 2018 10:50

layershifter mentioned this pull request Dec 11, 2018

Uncaught TypeError: cx is not a function in FelaTheme #497

Closed

layershifter mentioned this pull request May 27, 2019

chore(package): use @babel/register instead of ts-node #1395

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore(mixed): fix imports and introduce bundler test #570

chore(mixed): fix imports and introduce bundler test #570

layershifter commented Dec 6, 2018 •

edited

Loading

layershifter Dec 6, 2018

miroslavstastny Dec 10, 2018 •

edited

Loading

layershifter Dec 10, 2018

layershifter Dec 10, 2018

miroslavstastny Dec 10, 2018

layershifter Dec 10, 2018

kuzhelov Dec 10, 2018 •

edited

Loading

layershifter Dec 10, 2018

kuzhelov Dec 10, 2018

layershifter Dec 10, 2018

kuzhelov Dec 10, 2018

layershifter Dec 11, 2018

mnajdova Dec 11, 2018 •

edited

Loading

layershifter Dec 11, 2018

kuzhelov Dec 10, 2018

layershifter Dec 10, 2018

kuzhelov Dec 10, 2018

kuzhelov Dec 10, 2018 •

edited

Loading

layershifter Dec 11, 2018 •

edited

Loading

		@@ -0,0 +1,110 @@
		import replace from 'rollup-plugin-replace'

chore(mixed): fix imports and introduce bundler test #570

chore(mixed): fix imports and introduce bundler test #570

Conversation

layershifter commented Dec 6, 2018 • edited Loading

Choose a reason for hiding this comment

miroslavstastny Dec 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

kuzhelov Dec 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

mnajdova Dec 11, 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

kuzhelov Dec 10, 2018 • edited Loading

Choose a reason for hiding this comment

layershifter Dec 11, 2018 • edited Loading

Choose a reason for hiding this comment

layershifter commented Dec 6, 2018 •

edited

Loading

miroslavstastny Dec 10, 2018 •

edited

Loading

kuzhelov Dec 10, 2018 •

edited

Loading

mnajdova Dec 11, 2018 •

edited

Loading

kuzhelov Dec 10, 2018 •

edited

Loading

layershifter Dec 11, 2018 •

edited

Loading