Skip to content

Conversation

gribnoysup
Copy link
Collaborator

This still needs some clean up and testing but works already locally and is very close to being ready so opening it as a draft if someone wants to check out the changes or pull the branch and try it out locally (I'd suggest a fresh clone of the repository if you want to do this)

pre-commit was a leftover from one of the previous cleanups and started to
cause issues when hoisted to the root

bson was causing issues on compass start due to some bugs in old bson
versions
This doesn't play well with workspaces hoisting
… distribution plugins as Node.js modules

This is required so that Compass can resolve plugins locally correctly when running
in dev mode locally with workspaces hoisting
"nyc": "^15.0.0",
"peer-deps-externals-webpack-plugin": "^1.0.2",
"postcss-loader": "^2.1.6",
"pre-commit": "^1.1.3",
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

/cc @mcasimir I know that you were planning to clean it up in another PR, but it causes issues when using workspaces so I had to do this clean up here to be able to commit changes and I thought it would be easier to just do a clean up in the branch right away

@gribnoysup gribnoysup force-pushed the npm-workspaces branch 2 times, most recently from 2cd3196 to 23a63a7 Compare June 5, 2021 12:24
@gribnoysup gribnoysup force-pushed the npm-workspaces branch 2 times, most recently from d14d4bb to 15017dd Compare June 8, 2021 08:27
…k test config where to look for node modules to externalize

Without providing explicit options to 'externals', webpack bundles packages
like 'enzyme' which leads to test runtime failures
As dependencies are hoisted, electorn-rebuild that we often use for
testing plugins leaves native modules in the state that breaks the
following tests
@gribnoysup
Copy link
Collaborator Author

Okay, so CI is green now, so I'm marking this as "Ready for review", there are a few skipped tests though, I'll look into them in the meantime

@gribnoysup gribnoysup marked this pull request as ready for review June 8, 2021 15:17
Copy link
Member

@Anemy Anemy left a comment

Choose a reason for hiding this comment

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

lgtm - cool rebuild script! This is really nice for working on Compass.

I think the one thing to watch out for with this PR is a few bson packages are being updated. compass-crud from ^4.0.3 to ^4.4.0. We might want to double check things involving types and things like variable type conversions are happening how we expect/how they were previously.

@gribnoysup
Copy link
Collaborator Author

gribnoysup commented Jun 10, 2021

I think the one thing to watch out for with this PR is a few bson packages are being updated. compass-crud from ^4.0.3 to ^4.4.0

Reverted everything to 4.1.0 which seems to be the last version that we can safely use (based on NODE-2848)

@gribnoysup
Copy link
Collaborator Author

For some reason CI isn't triggering on the PR automatically, but I did it manually and it's all still green: https://github.com/mongodb-js/compass/actions/runs/922637764

@gribnoysup
Copy link
Collaborator Author

Planning to resolve conflicts after we merge the timeseries work, other than that I think this is ready to be merged!

@gribnoysup
Copy link
Collaborator Author

Okay, so this is synced with main now and should be ready to be merged. I also added more oses to github actions to see if tests are passing there (they might not just due to the differences between github machines and evergreen ones) because I can't submit this patch to evergreen manually, it's too big.

Another important disclaimer: this branch allows to run Compass locally with no issues, but produces a broken Compass build, this is due to changes in hadron-plugin-manager and hadron-style-manager in this branch that need to be published first before Compass will actually include them in the application build

Conflicts:
	packages/compass-aggregations/package.json
	packages/compass-auto-updates/package.json
	packages/compass-database/package.json
	packages/compass-loading/package.json
	packages/compass-schema-validation/package.json
	packages/compass-serverstats/package.json
	packages/compass-shell/package.json
	packages/compass-ssh-tunnel-status/package.json
@gribnoysup gribnoysup merged commit 0f2b435 into master Jun 15, 2021
@gribnoysup gribnoysup deleted the npm-workspaces branch June 15, 2021 17:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants