Skip to content
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

Repo Overhaul #607

Open
wants to merge 3 commits into
base: main-perspectives-merge-with-debug
Choose a base branch
from
Open

Conversation

nmn
Copy link
Collaborator

@nmn nmn commented Dec 5, 2024

Starting to overhaul the entire repo to make it as simple as possible.

Here are the notable changes so far:

Move to Bun

bun is an alternative to Node.js, NPM, rollup, Babel and Jest.
This one tool should replace all of those individual tools and be significantly faster too.

Migrate from Flow to Typescript

Compilation

bun understands Typescript natively without any custom configuration. It also supports all the of features that we used plugins with Rollup. It should be able to compile and bundle JS and TS and TSX code without any custom configuration out of the box.

Typesystem

Typescript is not as strict as Flow, but it is a lot easier to use. Most of what you learned with Flow will continue to work, but you'll just get fewer errors. There are few small changes to be aware of:

  • empty -> never
  • mixed -> unknown
  • $ReadOnly<Obj> -> Readonly<Obj>
  • $ReadOnlyArray<T> -> ReadonlyArray<T>
  • type X = {...Y, z: string} -> type X = Y & {z: string}
  • function foo<T:string>(){} -> function foo<T extends string>(){}
  • return (x: string); -> return x as string

Additionally:

  • you don't have to manually type out as my type annotations with Typescript
  • VS Code should work more reliably
  • Other editors, like Zed, should "just work"
  • There should be better error messages
  • There should be fewer bugs
  • Fewer breaking changes when upgrading

Mono-repo

The final big change is to create plugin folder at the top-level of the repo and move all the plugin folders under that. Additionally, every plugin folder (and other utility folders) now all have a package.json file with an explicit "name" for the package.

With this setup, you can always import code from another package using the name defined in the package.json file. I followed a consistent pattern of naming where a package like:

  • jgclark.NoteHelpers has the name @jgclark/note-helper

Any folder that did not have a username prefix now has an np. and @np/ prefixes respectively.

All of these imports "just work" for both Typescript and Bundling without manually setting up any aliases.

A lot more to do

This is a big overhaul, and I don't remember all of scripts that need to run. So I will need help getting it all to work.

@nmn nmn requested review from dwertheimer and jgclark and removed request for dwertheimer December 5, 2024 09:18
@jgclark
Copy link
Collaborator

jgclark commented Dec 5, 2024

O wow, this is a huge change. I'm really pleased to have your suggestions @nmn.
This doesn't feel like something to do under an isolated PR, particularly against a moving codebase and with a volunteer team. This is surely a project in its own right?

My immediate concern is npc which is literally key to every build and release. It's now stopped working for me following #604, and I don't know whether our CI checks cover whether that actually works. So I need some reassurance about that before I take any further action here.

FYI it's the first I've heard of bun, and I've never used typescript before.

Other questions that occur to me straight away:

  • what happens to our $FlowIgnore and $FlowFixme comments?
  • does ts have a way of ignoring a class of warning for a whole file?
  • what's the suggested tooling to use in VSCode to see and deal with ts warnings?

@nmn
Copy link
Collaborator Author

nmn commented Dec 5, 2024

what happens to our $FlowIgnore and $FlowFixme comments?

Typescript uses // @ts-ignore and // @ts-expect-error respectively. The second comment enforces that there is an error.

does ts have a way of ignoring a class of warning for a whole file?

The tsconfig.json has lots of config options and it's possible to enable or disable certain checks. For example, TS can be strict about warning about anything that is accidentally any, but you can configure it.

what's the suggested tooling to use in VSCode to see and deal with ts warnings?

VSCode has Typescript built-in so it should "just work". Outside of that I've updated the bun run typecheck to also list the type errors from the command line.


Yes, this is a very big change and maybe it won't work out specially as the code is constantly changing. There might be one other way to make this move in smaller steps that might be worth trying. I wanted to see how far I could get in a couple of hours and while a lot is fixed a lot still isn't.

@dwertheimer
Copy link
Collaborator

@aaronpoweruser Are you up to taking a look at this and maybe trying to get it working for your plugin as a test? This stuff is way outside of my expertise, so I don't think there's much I can do to move this forward.

@aaronpoweruser
Copy link
Collaborator

Sure I can take a look but we need to come up with a way to effectively test all the plugins.

@nmn
Copy link
Collaborator Author

nmn commented Dec 10, 2024

Most of the code is the same as before but the location of everything has changed.

  • Each plugin is now a package and can define tests locally within.
  • Each plugin can have different scripts.

I'll try to find some time and put up another PR which is less drastic and makes a small change to start off with. In the meantime if this works then great, otherwise just close this PR.

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.

4 participants