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

typescript re-write #57

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

typescript re-write #57

wants to merge 3 commits into from

Conversation

mmkal
Copy link

@mmkal mmkal commented Mar 14, 2025

Fixes #56

This replaces coffee/omelette.coffee with typescript/omelette.ts

Some notes:

  • The implementation aims to match the runtime output as closely as possible to src/omelette.js, rather than trying to match the coffee source to typescript
  • It aims to match the type definition output to the existing definitelytyped module: https://github.com/DefinitelyTyped/DefinitelyTyped/blob/26b0538e9b3cd7498827609a59c09eb5a2fdd58b/types/omelette/index.d.ts
  • Neither of the above was possible to match 100%, so some help testing would be appreciated! I didn't change .travis.yml, I haven't used it in a few years, not sure some extra bits are needed
  • added typescript as a dev dependency
  • Which also required using a package manager to install (I used pnpm but can switch to npm or yarn if preferred)
  • Which also required adding a .gitignore containing node_modules and a pnpm-lock.yaml
  • added a tsconfig.json (strict is enabled though this required using one non-null assertion (this.word!)
  • added a build script to package.json
  • added the types field to package.json
  • added a test/test-d.ts file to make sure the types don't regress, copying from https://github.com/DefinitelyTyped/DefinitelyTyped/blob/26b0538e9b3cd7498827609a59c09eb5a2fdd58b/types/omelette/omelette-tests.ts
  • added a test script to package.json which currently just compiles the types test file

Stuff I didn't change:

  • any CI workflows (I noticed there are both github actions and travis)
  • didn't remove the src/omelette.js folder, even though it's generated from typescript/omelette.ts - keeping under source control is somewhat unconventional but it's equivalent to how it was with Coffeescript
  • didn't aim to improve any implementations or type definitions along the way - thought there'd be more value in a port that matches the published version as closely as possible
  • any consolidation with deno
  • any file/folder organization patterns. It's somewhat unusual to have the compiled output live in src/, usually that would be for the "source" code (typescript/coffee), and the output would be in dist/ or lib/. But not a big deal so I left it.

Any/all of those could come after, but I don't see them as problems per se - esp if updates to this repo aren't too frequent and releases are manual.

Review notes:

  • it might be better to look at the diff to src/omelette.js rather than typescript/omelette.ts (you'll need to "show large diffs" and will want to ignore whitespace changes if viewing on github)

Testing:

  • autocompletion is obviously v hard to test on a CI runner, but I tried it out by linking locally to my trpc-cli which uses omelette to add completions to a CLI built from a trpc router (commander instance under the hood). Types and runtime look good, but again help testing would be great!

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.

Convert to typescript?
1 participant