-
Notifications
You must be signed in to change notification settings - Fork 7
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
build(election-manager): add to the yarn workspace #139
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Something changed such that the `assert` module that gets bundled no longer has the `strict` property, so I switched it to `strictEqual` instead of `strict.equal`.
benadida
approved these changes
Jan 12, 2021
eventualbuddha
added a commit
that referenced
this pull request
Jan 13, 2021
I tried to get all the apps and libraries under a single yarn workspace and found I could not do it. This message describes the difficulties I had. ### There might be a problem with the project dependency tree. `react-scripts` does some preflight checks to ensure that there isn't another resolvable copy of some of the packages it needs that might conflict with the versions it wants. When it finds one, [it bails with an error message](facebook/create-react-app#4296). Trying to put multiple `react-scripts`-using projects into the same yarn workspace means that yarn will hoist shared dependencies, such as `babel-eslint`, to the root. Thus, these packages become visible to the preflight check and it aborts. To work around this, you can use the [`nohoist`](https://classic.yarnpkg.com/blog/2018/02/15/nohoist/) option to tell yarn which packages not to hoist. This sort of worked, but there were subtle issues remaining. ### Versions changing when adding to the workspace I got BMD and ballot-encoder into the workspace together, but when I added election-manager [it broke BMD](#139). I still don't know exactly why, but I believe something changed the versions of some packages that BMD was using in a way that caused the tests to fail. ### Incompatible type declarations Sometimes multiple packages reference the same types, i.e. for a mutual dependency. However, sometimes they'd end up using different (and subtly incompatible) versions of the same `@types/` package. When this happened, the best recourse was to use another yarn workspaces feature: `resolutions`. This allows you to configure what version packages should resolve to. This fixed most of the type incompatibility issues, but I suspect it caused another issue. ### yarn resolved an older copy than requested I noticed some crashes due to an API incompatibility. Specifically, one of the `@babel/` packages required another one in the `^7.12.0` range, but yarn hooked it up with `7.8.6`. Why? I don't know. Perhaps it was the `resolutions` stuff. I was only able to work around this by finding the entry in the `yarn.lock` file, deleting it, and re-running `yarn`. ### Trying out alternatives I tried both npm v7 and pnpm. npm v7 has support for workspaces similar to yarn, but does not yet have support for configuring hoisting behavior. Lacking hoisting control meant it hoisted everything it could, which made `react-scripts` angry. Thus I couldn't use npm v7. Previous versions of pnpm did not hoist anything, and still was able to share dependencies by symlinking. Though it does hoist by default now, that behavior can be turned off. So I added `hoist = false` to `.npmrc` and then started trying to run everything to determine what would break without hoisting. It turns out that a good number of packages have implicit dependencies that hoisting allowed them to access, but that could not be accessed in a non-hoisted `node_modules` layout. pnpm has a hook called `readPackage` that allows altering the package data for any npm package using any custom logic you want. This allows us to act as if these packages with implicit dependencies actually had those dependencies listed explicitly. In my testing, this has yielded a good balance of correctness and functionality. So pnpm seems to work and I think is flexible enough to meet our needs going forward.
eventualbuddha
added a commit
that referenced
this pull request
Jan 14, 2021
I tried to get all the apps and libraries under a single yarn workspace and found I could not do it. This message describes the difficulties I had. `react-scripts` does some preflight checks to ensure that there isn't another resolvable copy of some of the packages it needs that might conflict with the versions it wants. When it finds one, [it bails with an error message](facebook/create-react-app#4296). Trying to put multiple `react-scripts`-using projects into the same yarn workspace means that yarn will hoist shared dependencies, such as `babel-eslint`, to the root. Thus, these packages become visible to the preflight check and it aborts. To work around this, you can use the [`nohoist`](https://classic.yarnpkg.com/blog/2018/02/15/nohoist/) option to tell yarn which packages not to hoist. This sort of worked, but there were subtle issues remaining. I got BMD and ballot-encoder into the workspace together, but when I added election-manager [it broke BMD](#139). I still don't know exactly why, but I believe something changed the versions of some packages that BMD was using in a way that caused the tests to fail. Sometimes multiple packages reference the same types, i.e. for a mutual dependency. However, sometimes they'd end up using different (and subtly incompatible) versions of the same `@types/` package. When this happened, the best recourse was to use another yarn workspaces feature: `resolutions`. This allows you to configure what version packages should resolve to. This fixed most of the type incompatibility issues, but I suspect it caused another issue. I noticed some crashes due to an API incompatibility. Specifically, one of the `@babel/` packages required another one in the `^7.12.0` range, but yarn hooked it up with `7.8.6`. Why? I don't know. Perhaps it was the `resolutions` stuff. I was only able to work around this by finding the entry in the `yarn.lock` file, deleting it, and re-running `yarn`. I tried both npm v7 and pnpm. npm v7 has support for workspaces similar to yarn, but does not yet have support for configuring hoisting behavior. Lacking hoisting control meant it hoisted everything it could, which made `react-scripts` angry. Thus I couldn't use npm v7. Previous versions of pnpm did not hoist anything, and still was able to share dependencies by symlinking. Though it does hoist by default now, that behavior can be turned off. So I added `hoist = false` to `.npmrc` and then started trying to run everything to determine what would break without hoisting. It turns out that a good number of packages have implicit dependencies that hoisting allowed them to access, but that could not be accessed in a non-hoisted `node_modules` layout. pnpm has a hook called `readPackage` that allows altering the package data for any npm package using any custom logic you want. This allows us to act as if these packages with implicit dependencies actually had those dependencies listed explicitly. In my testing, this has yielded a good balance of correctness and functionality. So pnpm seems to work and I think is flexible enough to meet our needs going forward.
eventualbuddha
added a commit
that referenced
this pull request
Jan 14, 2021
I tried to get all the apps and libraries under a single yarn workspace and found I could not do it. This message describes the difficulties I had. `react-scripts` does some preflight checks to ensure that there isn't another resolvable copy of some of the packages it needs that might conflict with the versions it wants. When it finds one, [it bails with an error message](facebook/create-react-app#4296). Trying to put multiple `react-scripts`-using projects into the same yarn workspace means that yarn will hoist shared dependencies, such as `babel-eslint`, to the root. Thus, these packages become visible to the preflight check and it aborts. To work around this, you can use the [`nohoist`](https://classic.yarnpkg.com/blog/2018/02/15/nohoist/) option to tell yarn which packages not to hoist. This sort of worked, but there were subtle issues remaining. I got BMD and ballot-encoder into the workspace together, but when I added election-manager [it broke BMD](#139). I still don't know exactly why, but I believe something changed the versions of some packages that BMD was using in a way that caused the tests to fail. Sometimes multiple packages reference the same types, i.e. for a mutual dependency. However, sometimes they'd end up using different (and subtly incompatible) versions of the same `@types/` package. When this happened, the best recourse was to use another yarn workspaces feature: `resolutions`. This allows you to configure what version packages should resolve to. This fixed most of the type incompatibility issues, but I suspect it caused another issue. I noticed some crashes due to an API incompatibility. Specifically, one of the `@babel/` packages required another one in the `^7.12.0` range, but yarn hooked it up with `7.8.6`. Why? I don't know. Perhaps it was the `resolutions` stuff. I was only able to work around this by finding the entry in the `yarn.lock` file, deleting it, and re-running `yarn`. I tried both npm v7 and pnpm. npm v7 has support for workspaces similar to yarn, but does not yet have support for configuring hoisting behavior. Lacking hoisting control meant it hoisted everything it could, which made `react-scripts` angry. Thus I couldn't use npm v7. Previous versions of pnpm did not hoist anything, and still was able to share dependencies by symlinking. Though it does hoist by default now, that behavior can be turned off. So I added `hoist = false` to `.npmrc` and then started trying to run everything to determine what would break without hoisting. It turns out that a good number of packages have implicit dependencies that hoisting allowed them to access, but that could not be accessed in a non-hoisted `node_modules` layout. pnpm has a hook called `readPackage` that allows altering the package data for any npm package using any custom logic you want. This allows us to act as if these packages with implicit dependencies actually had those dependencies listed explicitly. In my testing, this has yielded a good balance of correctness and functionality. So pnpm seems to work and I think is flexible enough to meet our needs going forward.
eventualbuddha
added a commit
that referenced
this pull request
Jan 14, 2021
I tried to get all the apps and libraries under a single yarn workspace and found I could not do it. This message describes the difficulties I had. `react-scripts` does some preflight checks to ensure that there isn't another resolvable copy of some of the packages it needs that might conflict with the versions it wants. When it finds one, [it bails with an error message](facebook/create-react-app#4296). Trying to put multiple `react-scripts`-using projects into the same yarn workspace means that yarn will hoist shared dependencies, such as `babel-eslint`, to the root. Thus, these packages become visible to the preflight check and it aborts. To work around this, you can use the [`nohoist`](https://classic.yarnpkg.com/blog/2018/02/15/nohoist/) option to tell yarn which packages not to hoist. This sort of worked, but there were subtle issues remaining. I got BMD and ballot-encoder into the workspace together, but when I added election-manager [it broke BMD](#139). I still don't know exactly why, but I believe something changed the versions of some packages that BMD was using in a way that caused the tests to fail. Sometimes multiple packages reference the same types, i.e. for a mutual dependency. However, sometimes they'd end up using different (and subtly incompatible) versions of the same `@types/` package. When this happened, the best recourse was to use another yarn workspaces feature: `resolutions`. This allows you to configure what version packages should resolve to. This fixed most of the type incompatibility issues, but I suspect it caused another issue. I noticed some crashes due to an API incompatibility. Specifically, one of the `@babel/` packages required another one in the `^7.12.0` range, but yarn hooked it up with `7.8.6`. Why? I don't know. Perhaps it was the `resolutions` stuff. I was only able to work around this by finding the entry in the `yarn.lock` file, deleting it, and re-running `yarn`. I tried both npm v7 and pnpm. npm v7 has support for workspaces similar to yarn, but does not yet have support for configuring hoisting behavior. Lacking hoisting control meant it hoisted everything it could, which made `react-scripts` angry. Thus I couldn't use npm v7. Previous versions of pnpm did not hoist anything, and still was able to share dependencies by symlinking. Though it does hoist by default now, that behavior can be turned off. So I added `hoist = false` to `.npmrc` and then started trying to run everything to determine what would break without hoisting. It turns out that a good number of packages have implicit dependencies that hoisting allowed them to access, but that could not be accessed in a non-hoisted `node_modules` layout. pnpm has a hook called `readPackage` that allows altering the package data for any npm package using any custom logic you want. This allows us to act as if these packages with implicit dependencies actually had those dependencies listed explicitly. In my testing, this has yielded a good balance of correctness and functionality. So pnpm seems to work and I think is flexible enough to meet our needs going forward.
eventualbuddha
added a commit
that referenced
this pull request
Jan 14, 2021
I tried to get all the apps and libraries under a single yarn workspace and found I could not do it. This message describes the difficulties I had. `react-scripts` does some preflight checks to ensure that there isn't another resolvable copy of some of the packages it needs that might conflict with the versions it wants. When it finds one, [it bails with an error message](facebook/create-react-app#4296). Trying to put multiple `react-scripts`-using projects into the same yarn workspace means that yarn will hoist shared dependencies, such as `babel-eslint`, to the root. Thus, these packages become visible to the preflight check and it aborts. To work around this, you can use the [`nohoist`](https://classic.yarnpkg.com/blog/2018/02/15/nohoist/) option to tell yarn which packages not to hoist. This sort of worked, but there were subtle issues remaining. I got BMD and ballot-encoder into the workspace together, but when I added election-manager [it broke BMD](#139). I still don't know exactly why, but I believe something changed the versions of some packages that BMD was using in a way that caused the tests to fail. Sometimes multiple packages reference the same types, i.e. for a mutual dependency. However, sometimes they'd end up using different (and subtly incompatible) versions of the same `@types/` package. When this happened, the best recourse was to use another yarn workspaces feature: `resolutions`. This allows you to configure what version packages should resolve to. This fixed most of the type incompatibility issues, but I suspect it caused another issue. I noticed some crashes due to an API incompatibility. Specifically, one of the `@babel/` packages required another one in the `^7.12.0` range, but yarn hooked it up with `7.8.6`. Why? I don't know. Perhaps it was the `resolutions` stuff. I was only able to work around this by finding the entry in the `yarn.lock` file, deleting it, and re-running `yarn`. I tried both npm v7 and pnpm. npm v7 has support for workspaces similar to yarn, but does not yet have support for configuring hoisting behavior. Lacking hoisting control meant it hoisted everything it could, which made `react-scripts` angry. Thus I couldn't use npm v7. Previous versions of pnpm did not hoist anything, and still was able to share dependencies by symlinking. Though it does hoist by default now, that behavior can be turned off. So I added `hoist = false` to `.npmrc` and then started trying to run everything to determine what would break without hoisting. It turns out that a good number of packages have implicit dependencies that hoisting allowed them to access, but that could not be accessed in a non-hoisted `node_modules` layout. pnpm has a hook called `readPackage` that allows altering the package data for any npm package using any custom logic you want. This allows us to act as if these packages with implicit dependencies actually had those dependencies listed explicitly. In my testing, this has yielded a good balance of correctness and functionality. So pnpm seems to work and I think is flexible enough to meet our needs going forward.
eventualbuddha
added a commit
that referenced
this pull request
Jan 14, 2021
I tried to get all the apps and libraries under a single yarn workspace and found I could not do it. This message describes the difficulties I had. `react-scripts` does some preflight checks to ensure that there isn't another resolvable copy of some of the packages it needs that might conflict with the versions it wants. When it finds one, [it bails with an error message](facebook/create-react-app#4296). Trying to put multiple `react-scripts`-using projects into the same yarn workspace means that yarn will hoist shared dependencies, such as `babel-eslint`, to the root. Thus, these packages become visible to the preflight check and it aborts. To work around this, you can use the [`nohoist`](https://classic.yarnpkg.com/blog/2018/02/15/nohoist/) option to tell yarn which packages not to hoist. This sort of worked, but there were subtle issues remaining. I got BMD and ballot-encoder into the workspace together, but when I added election-manager [it broke BMD](#139). I still don't know exactly why, but I believe something changed the versions of some packages that BMD was using in a way that caused the tests to fail. Sometimes multiple packages reference the same types, i.e. for a mutual dependency. However, sometimes they'd end up using different (and subtly incompatible) versions of the same `@types/` package. When this happened, the best recourse was to use another yarn workspaces feature: `resolutions`. This allows you to configure what version packages should resolve to. This fixed most of the type incompatibility issues, but I suspect it caused another issue. I noticed some crashes due to an API incompatibility. Specifically, one of the `@babel/` packages required another one in the `^7.12.0` range, but yarn hooked it up with `7.8.6`. Why? I don't know. Perhaps it was the `resolutions` stuff. I was only able to work around this by finding the entry in the `yarn.lock` file, deleting it, and re-running `yarn`. I tried both npm v7 and pnpm. npm v7 has support for workspaces similar to yarn, but does not yet have support for configuring hoisting behavior. Lacking hoisting control meant it hoisted everything it could, which made `react-scripts` angry. Thus I couldn't use npm v7. Previous versions of pnpm did not hoist anything, and still was able to share dependencies by symlinking. Though it does hoist by default now, that behavior can be turned off. So I added `hoist = false` to `.npmrc` and then started trying to run everything to determine what would break without hoisting. It turns out that a good number of packages have implicit dependencies that hoisting allowed them to access, but that could not be accessed in a non-hoisted `node_modules` layout. pnpm has a hook called `readPackage` that allows altering the package data for any npm package using any custom logic you want. This allows us to act as if these packages with implicit dependencies actually had those dependencies listed explicitly. In my testing, this has yielded a good balance of correctness and functionality. So pnpm seems to work and I think is flexible enough to meet our needs going forward.
eventualbuddha
added a commit
that referenced
this pull request
Jan 19, 2021
I tried to get all the apps and libraries under a single yarn workspace and found I could not do it. This message describes the difficulties I had. `react-scripts` does some preflight checks to ensure that there isn't another resolvable copy of some of the packages it needs that might conflict with the versions it wants. When it finds one, [it bails with an error message](facebook/create-react-app#4296). Trying to put multiple `react-scripts`-using projects into the same yarn workspace means that yarn will hoist shared dependencies, such as `babel-eslint`, to the root. Thus, these packages become visible to the preflight check and it aborts. To work around this, you can use the [`nohoist`](https://classic.yarnpkg.com/blog/2018/02/15/nohoist/) option to tell yarn which packages not to hoist. This sort of worked, but there were subtle issues remaining. I got BMD and ballot-encoder into the workspace together, but when I added election-manager [it broke BMD](#139). I still don't know exactly why, but I believe something changed the versions of some packages that BMD was using in a way that caused the tests to fail. Sometimes multiple packages reference the same types, i.e. for a mutual dependency. However, sometimes they'd end up using different (and subtly incompatible) versions of the same `@types/` package. When this happened, the best recourse was to use another yarn workspaces feature: `resolutions`. This allows you to configure what version packages should resolve to. This fixed most of the type incompatibility issues, but I suspect it caused another issue. I noticed some crashes due to an API incompatibility. Specifically, one of the `@babel/` packages required another one in the `^7.12.0` range, but yarn hooked it up with `7.8.6`. Why? I don't know. Perhaps it was the `resolutions` stuff. I was only able to work around this by finding the entry in the `yarn.lock` file, deleting it, and re-running `yarn`. I tried both npm v7 and pnpm. npm v7 has support for workspaces similar to yarn, but does not yet have support for configuring hoisting behavior. Lacking hoisting control meant it hoisted everything it could, which made `react-scripts` angry. Thus I couldn't use npm v7. Previous versions of pnpm did not hoist anything, and still was able to share dependencies by symlinking. Though it does hoist by default now, that behavior can be turned off. So I added `hoist = false` to `.npmrc` and then started trying to run everything to determine what would break without hoisting. It turns out that a good number of packages have implicit dependencies that hoisting allowed them to access, but that could not be accessed in a non-hoisted `node_modules` layout. pnpm has a hook called `readPackage` that allows altering the package data for any npm package using any custom logic you want. This allows us to act as if these packages with implicit dependencies actually had those dependencies listed explicitly. In my testing, this has yielded a good balance of correctness and functionality. So pnpm seems to work and I think is flexible enough to meet our needs going forward.
Obviated by #146. |
eventualbuddha
deleted the
build/election-manager/add-to-yarn-workspace
branch
January 20, 2021 17:10
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Something changed such that the
assert
module that gets bundled no longer has thestrict
property, so I switched it tostrictEqual
instead ofstrict.equal
.