Skip to content

Fix links stripped from docs during Typedoc link resolution #1304

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

Closed
pokey opened this issue Mar 13, 2023 · 2 comments
Closed

Fix links stripped from docs during Typedoc link resolution #1304

pokey opened this issue Mar 13, 2023 · 2 comments
Labels
documentation Improvements or additions to documentation
Milestone

Comments

@pokey
Copy link
Member

pokey commented Mar 13, 2023

As part of #1281, we upgraded Typedoc to 0.23.x. That version of Typedoc includes a new link resolution algorithm for JSDocs that include {@link ...} links. Unfortunately, that new algorithm doesn't match the way that VSCode resolves links, resulting in links being dropped in our auto-generated API documentation. These broken links are indicated in #942 (comment)

This issue should be fixed on 0.24.0 (see TypeStrong/typedoc#2197 and TypeStrong/typedoc#2141), so we should try to upgrade.

I made an initial attempt to upgrade to 0.24.0, but couldn't get things working. A couple plugins were outdated, and it looks like cross-package links broke. When I tried to fix cross-package links using the new packages setting designed for monorepos, I got the following output and gave up:

[INFO] Starting the development server...
[info] Converting project at /Users/pokey/src/cursorless/packages/cheatsheet
[error] Unable to find any entry points. Make sure TypeDoc can find your tsconfig
[info] Converting project at /Users/pokey/src/cursorless/packages/cheatsheet-local
[info] Converting project at /Users/pokey/src/cursorless/packages/common
[info] Converting project at /Users/pokey/src/cursorless/packages/cursorless-engine
[info] Converting project at /Users/pokey/src/cursorless/packages/cursorless-org
[info] Converting project at ./
[error] Project at ./ has entryPointStrategy set to packages, but nested packages are not supported.
[info] Converting project at /Users/pokey/src/cursorless/packages/cursorless-vscode
[info] Converting project at /Users/pokey/src/cursorless/packages/cursorless-vscode-core
[info] Converting project at /Users/pokey/src/cursorless/packages/cursorless-vscode-e2e
[info] Converting project at /Users/pokey/src/cursorless/packages/meta-updater
[info] Converting project at /Users/pokey/src/cursorless/packages/test-harness
[info] Converting project at /Users/pokey/src/cursorless/packages/vscode-common
[info] Merging converted projects
[error] Failed to convert one or more packages, result will not be merged together.

Here's a tag with my attempt for future use. Should be able to:

git fetch
git switch -c typedoc-24-0 pokey/initial-failed-typedoc-0-24-0-attempt
pnpm up -rL '*typedoc*'

I do wonder how much value these api docs are providing, though. We're not really using typedoc as intended: it's supposed to be for documenting api's for public libraries, and we're trying to use it to document all of our internal code for our contributors, where arguably it might be better to just let our contributors have guided walk throughs of the source code itself

@pokey pokey added the documentation Improvements or additions to documentation label Mar 13, 2023
@pokey pokey added this to the Short list milestone Mar 13, 2023
@pokey pokey mentioned this issue Mar 13, 2023
55 tasks
pokey added a commit that referenced this issue Mar 21, 2023
- closes #945
- closes #1044
- depends on #1322
- depends on #1327

## Checklist

- [x] Split into smaller PRs for ease of review
- [x] Try DX tests again
#1289
- [x] 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)
- [x] File issue for speeding up cheatsheet / making it not optional
locally #1307
- [x] Set package manager in package.json
https://nodejs.org/api/packages.html#packagemanager
- [x] Re-add lint rule to prevent api types from importing anything
- [x] Try local install
  - [x] Make sure cheatsheet works
- [x] Make sure local extension build depends on building cheatsheet, or
allows no cheatsheet
- [x] Figure out why we're getting so many extra `internal` modules
- [x] Update #931 to
indicate we're now patching instead of swizzling
- [x] Make sure lint rules are actually running in CI
- [x] Check that we've addressed everything in
#945
- [x] Incorporate changes from
#1166
- [x] ~~Make sure this PR doesn't break doc links; see
#942 (comment)
There's no way to avoid these links getting broken if we upgrade
typedoc; filed #1304
to track
- [x] Revert #1284
- [x] I have added
[tests](https://www.cursorless.org/docs/contributing/test-case-recorder/)
- [x] I have not broken the cheatsheet
- [x] Generate cursorless-vscode package.json automatically, and change
the name we use to `@cursorless/cursorless-vscode` in the `package.json`
checked into source control
- [x] Be sure to update meta-updater to check for that instead of name
`cursorless` (see fixme in its `index.ts`)

## Desiderata

- From #1289

### Correctly flagging errors

- [x] `D1`: It is a compile error to import external packages that are
not listed in `package.json`
   - [x] `D1.1`: `vscode` (for some reason this one behaves strangely)
   - [x] `D1.2`: Other packages
- [x] `D2`: It is **not** a compile error to import from local packages
that you depend on, using our preferred syntax (eg `@cursorless/foo`)
- [x] `D3`: It is a compile error to import from local packages that you
depend on, **not** using our preferred syntax (eg `../foo`)
- [x] `D4`: It is a compile error to import from local packages that you
do **not** depend on, either
- [x] `D4.1`: using our non-preferred syntax (eg `../packages/foo`), or
  - [x] `D4.2`: using preferred syntax (eg `@cursorless/foo`)
- [ ] `D5`: It is a compile error to import from anything other than
`index.ts` in another module
  - [x] `D5.1`: **no** `@cursorless/foo/bar`
  - [x] `D5.2`: **no** `../foo/bar`
  - [x] `a`: Even if you depend on the module
- [ ] `b`: Even if `bar` was re-exported in `foo/index.ts`. In that case
it should be required to import `@cursorless/foo`. Untested, but
probably works, and I ran out of steam 😅
- [x] `D6`: It is **not** a compile error to import external packages
listed in `package.json`
   - [x] `D6.1`: `vscode` (for some reason this one behaves strangely)
   - [x] `D6.2`: Other packages

### Auto-import
- [x] `D7`: Auto-import doesn't import from external packages not listed
in `package.json`
   - [x] `D7.1`: `vscode` (for some reason this one behaves strangely)
   - [x] `D7.2`: Other packages
- [x] `D8`: Auto-import imports from external packages listed in
`package.json`
   - [x] `D8.1`: `vscode` (for some reason this one behaves strangely)
   - [x] `D8.2`: Other packages
- [x] `D9`: Auto-import imports from local packages that you depend on,
using our preferred syntax (eg `@cursorless/foo`)
- [x] `D10`: Auto-import doesn't import from local packages that you
don't depend on
- [x] `D11`: Auto-import doesn't import from anything other than
`index.ts` in another package (eg **no** `@cursorless/foo/bar`)
   - [x] `D11.1`: When you depend on the package
   - [x] `D11.2`: When you don't depend on the package

### Other DX
- [x] `D12`: Find references across projects when no file from the
referencing project is open
- [x] `D13`: Rename across projects when no file from the referencing
project is open
- [x] `D14`: Jump to definition across projects
- [ ] `D15`: It is easy to move a file from one package to another
- [ ] `D16`: It is easy to create a new package
- [ ] `D17`: Breakpoints work in Cursorless extension
- [x] `D18`: Breakpoints work on Cursorless root website
- [ ] `D19`: Breakpoints work on Cursorless docs website
@pokey pokey modified the milestones: Short list, Long list Apr 12, 2023
@Gerrit0
Copy link

Gerrit0 commented Apr 15, 2023

Unfortunately, that new algorithm doesn't match the way that VSCode resolves links

Just a note that TypeDoc 0.22's resolution didn't match either, it just happened that a global search for something named "X" is close enough for most projects that most didn't notice.

@pokey pokey mentioned this issue Aug 8, 2023
3 tasks
@pokey
Copy link
Member Author

pokey commented Aug 8, 2023

See #1749

@pokey pokey closed this as not planned Won't fix, can't repro, duplicate, stale Aug 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

No branches or pull requests

2 participants