Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move to mono repo structure #2326

Merged
merged 1 commit into from Jan 22, 2019

Conversation

Projects
None yet
4 participants
@imbhargav5
Copy link
Member

imbhargav5 commented Jan 14, 2019

This is very much a work in progress PR and we will track mono repo progress here.
TODO

  • Documentation
    • Update contribution and setup guidelines
    • Update publish process
  • Fix test:native
  • Fix test:primitives
  • Fix husky
  • TBD
  • Make sandbox another package and get it to work
  • Make Readme appear on npm page

@imbhargav5 imbhargav5 added the WIP label Jan 14, 2019

@imbhargav5

This comment has been minimized.

Copy link
Member Author

imbhargav5 commented Jan 14, 2019

Right now I am having trouble running jest native tests. yarn test:native in the root of the project is all we need to get right. At least for now.

@mxstbr

This comment has been minimized.

Copy link
Member

mxstbr commented Jan 15, 2019

TODO

  • Documentation
    • Update contribution and setup guidelines
    • Update publish process
  • Fix test:native
  • Fix test:primitives
  • Fix husky
  • TBD
@mxstbr

This comment has been minimized.

Copy link
Member

mxstbr commented Jan 15, 2019

According to this SO answer, one needs to set preset: 'react-native' in the Jest options to make it work: https://stackoverflow.com/questions/47269957/cannot-find-module-from-react-native-implementation-js

After doing that, I get a different error saying

$ jest -c scripts/jest/config.native.js
● Validation Error:

  Module <rootDir>/node_modules/react-native/jest/assetFileTransformer.js in the transform option was not found.
         <rootDir> is: /Users/mxstbr/projects/styled-components/styled-components/packages/styled-components

  Configuration Documentation:
  https://jestjs.io/docs/configuration.html

According to this issue, it can be resolved by not hoisting dependencies? facebook/react-native#17469 Will try and investigate that.

@mxstbr

This comment has been minimized.

Copy link
Member

mxstbr commented Jan 15, 2019

Update: nohoisting did not work. I'm not sure how to solve that, /cc @SimenB do you have any idea how to make this work?

@SimenB

This comment has been minimized.

Copy link

SimenB commented Jan 15, 2019

Heh, I had a PR to RN merged just yesterday that should fix this: facebook/react-native#22972

In the meantime, the thing you need to make sure of is that react-native lives in rootDir/node_modules/react-native. And since you set rootDir to be packages/styled-components, react-native must be installed in that directory (until my PR is released). However, it needs react to be a sibling, so just hoisting react-native is not enough. This diff makes yarn test:native pass:

diff --git i/package.json w/package.json
index 958a1b7..8aee71e 100644
--- i/package.json
+++ w/package.json
@@ -81,12 +80,6 @@
     "prettier": "^1.15.2",
     "puppeteer": "^1.10.0",
     "raf": "^3.4.1",
-    "react": "^16.6.1",
-    "react-dom": "^16.6.1",
-    "react-frame-component": "^4.0.2",
-    "react-native": "^0.56.0",
-    "react-primitives": "^0.6.1",
-    "react-test-renderer": "^16.6.1",
     "rimraf": "^2.6.1",
     "rollup": "^0.66.5",
     "rollup-plugin-babel": "^3.0.4",
@@ -98,7 +91,13 @@
     "rollup-plugin-sourcemaps": "^0.4.2",
     "rollup-plugin-terser": "^3.0.0"
   },
-  "workspaces": [
-    "packages/*"
-  ]
+  "workspaces": {
+    "packages": [
+      "packages/*"
+    ],
+    "nohoist": [
+      "**/react-*",
+      "**/react-*/**"
+    ]
+  }
 }
diff --git i/packages/styled-components/package.json w/packages/styled-components/package.json
index 8aa9e25..18e8a27 100644
--- i/packages/styled-components/package.json
+++ w/packages/styled-components/package.json
@@ -82,6 +82,14 @@
     "react": ">= 16.3.0",
     "react-dom": ">= 16.3.0"
   },
+  "devDependencies": {
+    "react": "^16.6.1",
+    "react-dom": "^16.6.1",
+    "react-frame-component": "^4.0.2",
+    "react-native": "^0.56.0",
+    "react-primitives": "^0.6.1",
+    "react-test-renderer": "^16.6.1"
+  },
   "jest": {
     "testURL": "http://localhost",
     "clearMocks": true,
diff --git i/scripts/jest/config.native.js w/scripts/jest/config.native.js
index 0f8b62e..d7a5c29 100644
--- i/scripts/jest/config.native.js
+++ w/scripts/jest/config.native.js
@@ -1,8 +1,13 @@
 const baseConfig = require('./config.base');
 
-module.exports = Object.assign({}, baseConfig, {
+const copy = Object.assign({}, baseConfig);
+
+// react-native preset brings its own haste implementation
+delete copy.haste;
+
+module.exports = Object.assign(copy, {
   testRegex: 'src/native/test/.*.js$',
-  preset: '../../node_modules/react-native/jest/assetFileTransformer.js',
+  preset: 'react-native',
   testURL: 'http://localhost',
   testEnvironment: 'jsdom',
 });
@mxstbr

This comment has been minimized.

Copy link
Member

mxstbr commented Jan 15, 2019

Thank you so much @SimenB, you're the best!! 💯 Latest commit makes yarn run test:native pass, let's see if anything else fails. If not, all this needs is documentation updates from @imbhargav5 and we're good to go 👍

@SimenB

This comment has been minimized.

Copy link

SimenB commented Jan 15, 2019

Another thing you can do is probably copy that preset manually into your own config.native.js and just replace every <rootDir>/node_modules/* thing with a require.resolve(*). Then you should be able to avoid the hoisting.

I haven't tested this, though

@imbhargav5

This comment has been minimized.

Copy link
Member Author

imbhargav5 commented Jan 17, 2019

@SimenB @mxstbr Any idea why the primitives test is failing with

Cannot find module 'setupDevtools' from 'setup.js'

I looked at this thread, but I couldn't find a working solution.

facebook/jest#3822

@SimenB

This comment has been minimized.

Copy link

SimenB commented Jan 17, 2019

Probably because you have a custom haste config. I'm unable to find this PR on CI, so not sure what exactly fails: https://travis-ci.org/styled-components/styled-components/pull_requests

@imbhargav5 imbhargav5 force-pushed the feat-lerna branch 2 times, most recently from 854fc06 to 950947e Jan 17, 2019

@imbhargav5

This comment has been minimized.

Copy link
Member Author

imbhargav5 commented Jan 17, 2019

@mxstbr Made changes to documentation. I think very small things remain. Can you help me with the documentation and help me merge this quickly?

@imbhargav5 imbhargav5 removed the WIP label Jan 17, 2019

@mxstbr
Copy link
Member

mxstbr left a comment

Some small nitpick inline, will take a look at the docs!

Show resolved Hide resolved package.json Outdated
Show resolved Hide resolved packages/sandbox/README.md Outdated
Show resolved Hide resolved packages/sandbox/__tests__/sandbox.test.js Outdated
Show resolved Hide resolved packages/sandbox/lib/sandbox.js Outdated
@mxstbr

This comment has been minimized.

Copy link
Member

mxstbr commented Jan 17, 2019

Contribution guidelines look good from what I can tell (haven't tested it though), now we only need docs for the publishing process! I don't know how any of that works with lerna, so you'll have to tackle it

@imbhargav5 imbhargav5 force-pushed the feat-lerna branch from 2c5fdca to 64b0c6d Jan 17, 2019

@mxstbr
Copy link
Member

mxstbr left a comment

Getting close, great job! 💯 Another round of nitpick-y quick fixes inline, and we should do a test publish as a beta version to make sure nothing is broken.

Show resolved Hide resolved package.json Outdated
Show resolved Hide resolved CONTRIBUTING.md Outdated
Show resolved Hide resolved CONTRIBUTING.md Outdated
@mxstbr

This comment has been minimized.

Copy link
Member

mxstbr commented Jan 18, 2019

Let's make usre v4.1.4-alpha.0 works in a codesandbox, and then we should be ready to go if @probablyup approves!

@imbhargav5

This comment has been minimized.

Copy link
Member Author

imbhargav5 commented Jan 18, 2019

Yes @mxstbr Trying it right now. Still getting a hang of lerna right now. The weird bit is, if lerna fails to publish for some reason, it's quite hard to publish again. It bails out saying "there are no changes to publish".

@imbhargav5

This comment has been minimized.

Copy link
Member Author

imbhargav5 commented Jan 18, 2019

@mxstbr https://codesandbox.io/s/239vz16l90 works here.
I introduced a couple of commits just to get Lerna to publish it. We can revert the Readme commit if needed. But I think we need to put a Readme in the packages/styled-components folder as well for it to show on npm. Not sure if Lerna automatically does it 🤔 .

@mxstbr

This comment has been minimized.

Copy link
Member

mxstbr commented Jan 18, 2019

Document all the stuff you figure out so if anybody else runs into the same error we already know how to solve it!

I'm not worried about the commits at all, just leave 'em in here. I'm not sure what to do about the readme, maybe we can .gitignore the packages/s-c/readme.md and then copy the main readme to that folder prepublish? Would that be an acceptable solution?

@imbhargav5

This comment has been minimized.

Copy link
Member Author

imbhargav5 commented Jan 18, 2019

@mxstbr I think we are ready. I will just squash commits now. @probablyup What do you think?

Move to a mono-repo structure
wip

Add gitignore and edit rollup config

Update gitignore

Wip

WIP

Get rid of older json files and use js files to extend across pkgs later

Make test:native work again, thanks to @SimenB

No haste and get integration test to work

Update husky

Update package.json

Update travis and appveyor

Add lint script

Update sandbox

Update yarn version

Move eslint into sc package

Remove unnecessary files

Update lint-staged

Update CONTRIBUTING.md

Update version in lerna.json

Update contributing

Update scripts

Update contributing

Update contributing

Update contributing.md and publish script

Update contributing

v4.1.4-alpha.0

Update package json

Update lerna version

v4.1.4-alpha.0

v4.1.4-alpha.1

Independent version for sandbox and styled-components

Yarn instead of npm

Add readme to styled-components package to show up on npm

Publish

 - styled-components@4.1.4-alpha.2

Update prepublisOnly and contributing

Update gitignore

Publish

 - styled-components@4.1.4-alpha.3

Test without readme

Remove readme

Publish

 - styled-components@4.1.4-alpha.4

Copy readme prepublishOnly

Publish

 - styled-components@4.1.4-alpha.5

@imbhargav5 imbhargav5 force-pushed the feat-lerna branch from cb459c2 to 4aa2fdb Jan 18, 2019

@imbhargav5 imbhargav5 requested review from probablyup and mxstbr Jan 20, 2019

@mxstbr mxstbr referenced this pull request Jan 21, 2019

Closed

Use hooks #2342

0 of 4 tasks complete
@probablyup
Copy link
Contributor

probablyup left a comment

Seems alright! Good work!

@imbhargav5

This comment has been minimized.

Copy link
Member Author

imbhargav5 commented Jan 22, 2019

@mxstbr Shall we merge?

@mxstbr

mxstbr approved these changes Jan 22, 2019

Copy link
Member

mxstbr left a comment

Let's ship it! 🎉

@mxstbr mxstbr merged commit c3eedbe into master Jan 22, 2019

2 checks passed

bundlesize ./dist/styled-components.min.js: 15.79KB < maxSize 16KB (gzip)(124B larger than master, careful!)
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@mxstbr mxstbr deleted the feat-lerna branch Jan 22, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.