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

Update Lerna and implement Yarn workspaces #289

Merged
merged 2 commits into from
Oct 8, 2018
Merged

Update Lerna and implement Yarn workspaces #289

merged 2 commits into from
Oct 8, 2018

Conversation

silvenon
Copy link
Contributor

@silvenon silvenon commented Oct 7, 2018

I updated the docs and Travis to prefer Yarn because we ship yarn.lock and we have workspaces. But everything works with npm too.

I also increased the timeout for the loader test. I'll proceed by adding inline comments.

Closes #268.

@vercel
Copy link

vercel bot commented Oct 7, 2018

This pull request is automatically deployed with Now.
To access deployments, click Details below or on the icon next to each push.

@@ -5,5 +5,6 @@ node_js:
git:
depth: 5
cache:
yarn: true
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure if this is necessary, i.e. if it happens by default.

@@ -1,8 +1,8 @@
{
"lerna": "2.5.1",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Lerna v3 doesn't have this field.

"docs": "x0 docs",
"docs:build": "x0 build docs",
"docs:deploy": "npm run docs:build && now dist --public && now alias",
"test": "lerna run test",
"format": "prettier --no-semi --single-quote --write '{examples,packages}/**/*.js'",
"prepare": "lerna bootstrap && lerna link --force-local"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I removed the prepare script because lerna bootstrap in Lerna v3 triggers that script, so it causes an endless loop. It would be ideal if we could keep this, because then lerna bootstrap would happen automatically and contributors and Travis CI wouldn't have to know about it, so I'm open to ideas.

@@ -22,7 +22,7 @@
"unist-util-visit": "^1.3.0"
},
"devDependencies": {
"jest": "^22.4.3",
"jest": "^23.6.0",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Tests were failing for mdxast, updating Jest fixed it.

@silvenon silvenon force-pushed the workspaces branch 2 times, most recently from 48497b9 to 47ed6be Compare October 7, 2018 01:01
@silvenon
Copy link
Contributor Author

silvenon commented Oct 7, 2018

I'm having a hard time figuring out why tests are failing. I'll investigate tomorrow.

@silvenon silvenon force-pushed the workspaces branch 2 times, most recently from c5c75ca to f2d5a1f Compare October 7, 2018 10:35
before_script:
# https://github.com/lerna/lerna/issues/1457#issuecomment-399678173
- yarn lerna exec yarn install
- yarn lerna link --force-local
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe it's a Lerna + Linux issue, but lerna bootstrap seems to work only partially on Travis, while lerna exec yarn install works entirely. lerna/lerna#1457 (comment)

I'm adding this workaround for now, but I'll also report this issue to Lerna to see if they can figure it out.

@@ -51,4 +51,4 @@ const testFixture = (fixture, options = {}) => {

test('it loads markdown and returns a component', async () => {
await testFixture('fixture.md')
})
}, 10000)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Previously Node v8 tests were failing, but now Node v10 were failing too, or instead. This seems to fix it.

@silvenon silvenon force-pushed the workspaces branch 3 times, most recently from a80cbf3 to 9f64077 Compare October 7, 2018 11:14
@silvenon
Copy link
Contributor Author

silvenon commented Oct 7, 2018

I was trying to remove that .travis.yml part because it wasn't necessary when I tried it locally in a Docker image, but unfortunately it seems to be necessary.

Anyway, enough tinkering, tests are passing so it's ready.

Copy link
Member

@johno johno left a comment

Choose a reason for hiding this comment

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

📻

This is awesome, thanks!

@silvenon silvenon merged commit 69c7f38 into master Oct 8, 2018
@silvenon silvenon deleted the workspaces branch October 8, 2018 15:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants