Skip to content

Migrate to a pnpm monorepo #945

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
12 tasks done
Tracked by #1023
pokey opened this issue Sep 9, 2022 · 0 comments · Fixed by #1281
Closed
12 tasks done
Tracked by #1023

Migrate to a pnpm monorepo #945

pokey opened this issue Sep 9, 2022 · 0 comments · Fixed by #1281
Labels
code quality Improvements to code quality to discuss Plan to discuss at meet-up

Comments

@pokey
Copy link
Member

pokey commented Sep 9, 2022

The problem

All of the Cursorless code exists as part of the VSCode extension and has many references to vscode. We'd like to abstract away VSCode and turn Cursorless into a generic npm package to enable the following:

  • Extract cursorless core into a node.js server #435 to support Cursorless in other IDEs
  • Allow other applications (eg rango, MS Word plugin) to leverage the Cursorless engine
  • Embed Cursorless in Talon for OCR-based global hats
  • Support Cursorless VSCode on the web
  • Share code with our docs site, which would enable one source of truth between docs and code
  • Allow us to fake / mock pieces of VSCode to enable proper unit testing (today nearly all Cursorless tests are end-to-end)

The solution

We'd like to migrate to nx. Let's use pnpm

Experiments to run

  • When you do auto-import in VSCode, does it look at node_modules of the package containing the file you're doing auto-import from even if your VSCode workspace is opened to the root directory of the project?
  • When you do a rename, will it apply across package boundaries?

To discuss

  • What to do about type imports?

Todo

old We'll proceed as follows

@pokey pokey added the code quality Improvements to code quality label Sep 9, 2022
@pokey pokey added the to discuss Plan to discuss at meet-up label Feb 21, 2023
@pokey pokey changed the title Separate Cursorless into multiple parts Turn Cursorless into a pnpm workspace monorepo Feb 21, 2023
@pokey pokey changed the title Turn Cursorless into a pnpm workspace monorepo Turn Cursorless into a pnpm monorepo Feb 21, 2023
@pokey pokey changed the title Turn Cursorless into a pnpm monorepo Migrate to a pnpm monorepo Feb 21, 2023
AndreasArvidsson pushed a commit that referenced this issue Feb 23, 2023
)

This PR does a massive restructure of our directories in preparation for
splitting up into separate packages for #945. Here is the new layout:

<img width="286" alt="image"
src="https://user-images.githubusercontent.com/755842/220430713-56d211fe-043a-46ae-b802-449cd0693cd5.png">


## Checklist

- [x] I have added
[tests](https://www.cursorless.org/docs/contributing/test-case-recorder/)
- [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] I have not broken the cheatsheet

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
@pokey pokey mentioned this issue Mar 9, 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
code quality Improvements to code quality to discuss Plan to discuss at meet-up
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant