-
-
Notifications
You must be signed in to change notification settings - Fork 85
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
Bump docusaurus to 3.0.0-alpha.0
#1745
Conversation
"@docusaurus/mdx-loader", | ||
"@docusaurus/plugin-content-blog", | ||
"@docusaurus/preset-classic" | ||
"react-json-view" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We still need this peer dependency override until facebook/docusaurus#9116 is released
@@ -41,14 +41,11 @@ | |||
}, | |||
"pnpm": { | |||
"patchedDependencies": { | |||
"@docusaurus/theme-search-algolia@2.3.1": "patches/@[email protected]" | |||
"@docusaurus/theme-search-algolia@3.0.0-alpha.0": "patches/@[email protected]" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this patch seemed to apply cleanly, and still is having the intended effect in local testing, so that's a relief 😅
pnpm-lock.yaml
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@auscompgeek anything jump out at you here?
96e7596
to
58fbe93
Compare
extends: toPosixPath( | ||
path.join(pathFromPackageToRoot, "tsconfig.base.json"), | ||
), | ||
extends: getExtends(pathFromPackageToRoot, input.extends), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
need this because we are now extending docusaurus tsconfig in our docs site
- Fixes #1749 - Will do a `pnpm remove` on all our typedoc dependencies once #1745 is merged so we don't get massive merge conflicts on our `pnpm.lock` 😅 - Our website deploy has gone from more than 12 minutes to 1 min 20 sec 🙌 ## Checklist - [ ] I have added [tests](https://www.cursorless.org/docs/contributing/test-case-recorder/) - [ ] I have updated the [docs](https://github.com/cursorless-dev/cursorless/tree/main/docs) and [cheatsheet](https://github.com/cursorless-dev/cursorless/tree/main/cursorless-talon/src/cheatsheet) - [ ] I have not broken the cheatsheet
#1745 is almost impossible to review 😅 I'd suggest running `pnpm dedupe` on the PR but it turns out `main` has a whole bunch of duplicate transitive deps too, so running it against `main` first. ## Checklist - [ ] ~I have added [tests](https://www.cursorless.org/docs/contributing/test-case-recorder/)~ - [ ] ~I have updated the [docs](https://github.com/cursorless-dev/cursorless/tree/main/docs) and [cheatsheet](https://github.com/cursorless-dev/cursorless/tree/main/cursorless-talon/src/cheatsheet)~ - [ ] I have not broken the cheatsheet
9ec62c3
to
a7ac988
Compare
ok @auscompgeek I've merged your dedupe. Fwiw I just ran a dedupe on this branch and did a soft reset over your dedupe; didn't feel like fighting with merge conflicts in pnpm lockfile 😅 |
Yep makes sense, I think the end result should be the same however you slice it 😄 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nothing that leapt out as majorly alarming to me. Left some comments on the more curious new deps coming in (although the comments are a bit of a stream of consciousness).
A few cases where there's -1 +1 versions of a dep, largely different semver major versions (judging by the node engine constraints on them, I'm guessing ES6 conversions or something). A few cases where we have one fewer version of an existing dep though, which is good to see.
|
||
/@babel/[email protected]: | ||
resolution: {integrity: sha512-qt/YV149Jman/6AfmlxJ04LMIu8bMoyl3RB91yTFrxQmgbrSvQMy7cI8Q62FHx1t8wJ8B5fu0UDoLwHAhUo1QA==} | ||
/@babel/[email protected]: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting, we had two versions of @babel/core
. Looks like the old come was being pulled in by mdx
/@mdx-js/[email protected]: | ||
resolution: {integrity: sha512-AMxuLxPz2j5/6TpF/XSdKpQP1NlG0z11dFOlq+2IP/lSgl11GY8ji6S/rgsViN/L0BDvHvUMruRb7ub+24LUYA==} | ||
dependencies: | ||
'@babel/core': 7.12.9 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, mdx 1 had babel 7.12 pinned. Looks like it doesn't depend on babel any more - pulling in estree instead.
@@ -3886,7 +3820,11 @@ packages: | |||
engines: {node: '>=12.22.0'} | |||
dev: true | |||
|
|||
/@pnpm/config@16.6.4(@pnpm/[email protected])(@yarnpkg/[email protected])([email protected]): | |||
/@pnpm/config[email protected]: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
interesting, pulling in a second version of this, weird
'@pnpm/network.ca-file': 1.0.2 | ||
config-chain: 1.1.13 | ||
dev: true | ||
|
||
/@pnpm/npm-lifecycle@2.0.0([email protected]): | ||
/@pnpm/npm-conf@2.2.2: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
another second version too
/@szmarczak/[email protected]: | ||
resolution: {integrity: sha512-4BAffykYOgO+5nzBWYwE3W90sBgLJoUPRWWcL8wlyiM8IB8ipJz3UMJ9KXQd1RKQXpKp8Tutn80HZtWsu2u76w==} | ||
engines: {node: '>=10'} | ||
dependencies: | ||
defer-to-connect: 2.0.1 | ||
dev: true | ||
|
||
/@szmarczak/[email protected]: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
another one here too, but it's a different major version 🤷
/[email protected]: | ||
resolution: {integrity: sha512-Oj3cAGPCqOZX7Rz64Uny2GYAZNliQSqfbePrgAQ1wKAihYmCUnraBtJtKcGR4xz7wF+LoJC+ssFZvv5BgF9Igg==} | ||
engines: {node: '>=8'} | ||
/[email protected]: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
2 major versions, hmm
@@ -7115,6 +7100,11 @@ packages: | |||
resolution: {integrity: sha512-Gmy6FhYlCY7uOElZUSbxo2UCDH8owEk996gkbrpsgGtrJLM3J7jGxl9Ic7Qwwj4ivOE5AWZWRMecDdF7hqGjFA==} | |||
engines: {node: '>=10'} | |||
|
|||
/[email protected]: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
3 major versions...
is this what it says on the tin? 😂
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If it does what it says on the tin, then the fact that we pull in 3 major versions of a one-line function is pretty much the npm ecosystem in a nutshell 😂
@@ -7716,6 +7713,14 @@ packages: | |||
/[email protected]: | |||
resolution: {integrity: sha512-v1plID3y9r/lPhviJ1wrXpLeyUIGAZ2SHNYTEapm7/8A9nLPoyvVp3RK/EPFqn5kEznyWgYZNsRtYYIWbuG8KA==} | |||
engines: {node: '>=8'} | |||
dev: true | |||
|
|||
/[email protected]: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
a second major version here too.
resolution: {integrity: sha512-x8dy3RnvYdlUcPOjkEHqozhiwzKNSq7GcPuXFbnyMOCHxX8V3OgIg/pYuabl2sbUPfIJaeAQB7PMOK8DFIdoRA==} | ||
engines: {node: '>=12'} | ||
dependencies: | ||
type-fest: 1.4.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that's an interesting transitive dependency 😂
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ha yeah that's...interesting. Should type-fest every be anything other than dev dependency?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah I don't think this should be a non-dev dependency, but I haven't checked how it's actually being used here
/[email protected]: | ||
resolution: {integrity: sha512-Ci6bIfq/UgcxPTYa8dQQ5FY3BzKkT894bwXWXxC/zqs0XgMO2cT20CGkOqda7gZNkmK5VP4x89IGZ6K7hfbn3Q==} | ||
engines: {node: '>=8'} | ||
dependencies: | ||
map-age-cleaner: 0.1.3 | ||
mimic-fn: 3.1.0 | ||
dev: true | ||
|
||
/[email protected]: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wat
... oh, github diff didn't notice these already existed?
We have some security warnings on a couple packages that are dependencies of docusaurus, so I decided to upgrade to the latest to remove those dependencies. Docusaurus uses their nightly version on their own docs, and things seem to work for us on this version with no issue, so I think it's ok. In addition, we were violating their peer dependencies by using react 18; we now no longer need to do that. Also, this version allows us to use mdx v2, which has a lot of niceties that we'll probably want as we do our big docs refactor (#867)
Checklist