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.

Sign up for GitHub

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

Jump to bottom

Typescript migration #534

Merged
merged 53 commits into from
Jul 13, 2023
Merged

Typescript migration #534

merged 53 commits into from
Jul 13, 2023

Conversation

jwu910
Copy link
Owner

@jwu910 jwu910 commented Jun 20, 2023 •
edited

Description

Fixes #492

Seems like everything was migrated... need to test and double check all the dep updates that went in while this was split prior to cutting a new release (or look into any other features/fixes)

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update

Checklist:

  • I have read the CONTRIBUTING document.
  • My commits follow the commitizen commit convention
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.

jwu910 and others added 29 commits February 20, 2023 16:56
@jwu910
chore: Install typescript dependency
684ff2e
@jwu910
feat: migrate helpText to ts to test
947e9e6
@jwu910
chore: add tsc to build script
dda024e
@jwu910
chore: install type definitions
cb06af2
@jwu910
chore: Autogenerate package lock
bbbbb67
@jwu910
fix: rename interface to ts ext, add return types
6255435
@jwu910
chore: remove unused comment
00b9c93
@a-camarillo @jwu910
chore: 🤖 remove eslint
16b46e1 
Uninstalled eslint

✅ Closes: 501
@jwu910
chore: add missing tsconfig file
a9e876c
@jwu910
chore: upgrade prettier, check js and ts files
b56021c
@jwu910
chore: move index to src to preserve rootDir settings, remove babel f…
133107b 
…orm build steps
@jwu910
chore: remove babel from dev deps
891a890
@jwu910
chore(ci): add action for ci and nvm version for local development
09313ca
@jwu910
chore: auto format
20fd342
@jwu910
chore: update build asset directories for binaries
f468e8f
@jwu910
chore(ci): change github action ref to use event pull request head
2e8de60
@jwu910
chore(ci): change github action ref to use event pull request base
6bdee23
@Jimmy-Vu @jwu910
chore: 🤖 Uninstall danger.js from devDependency
e17f4da 
The danger.js package is uninstalled from the project along with its
related config files. .travis.yml is deleted.

BREAKING CHANGE: 🧨 N/A

✅ Closes: 493
@drewbrokke @jwu910
chore: 🤖 adds types for update-notifier
7d44b07
@drewbrokke @jwu910
refactor: 💡 adds types.ts
0dca081
@drewbrokke @jwu910
refactor: 💡 migrate git.js -> git.ts
ad850d9
@drewbrokke @jwu910
refactor: 💡 migrate state.js -> state.ts
0f4ce67
@drewbrokke @jwu910
refactor: 💡 migrate app.js -> app.ts
836beca
@drewbrokke @jwu910
refactor: 💡 migrate index.js -> index.ts
670524b 
✅ Closes: #492
@drewbrokke @jwu910
docs: ✏️ updates run instructions
530a6c1
@drewbrokke @jwu910
refactor: 💡 uses custom type to get rid of ts-ignore comment
ec2afc4
@drewbrokke @jwu910
chore: 🤖 package-lock.json
3edd954
@jwu910
!chore: bump min node version to account for new code
008aad7
@jwu910
chore: clean up comments, sort imports, update code per ts errors
d41aedf
@jwu910 jwu910 requested a review from kienD as a code owner June 20, 2023 04:22
@jwu910
Copy link
Owner Author

jwu910 commented Jun 20, 2023

Actually i'm having issues working through tsconfigs right now to make sure we can use update-notifier, it got bumped 4 major versions in one of these PRs lol, and getting esm vs cjs errors atm

@drewbrokke
Copy link
Collaborator

@jwu910 any details/steps to reproduce? I'd be happy to take a look if that would be helpful.

@jwu910
Copy link
Owner Author

jwu910 commented Jul 11, 2023

Hey @drewbrokke, that would be amazing.

So far i've seen this issue:

  1. npm run build (on latest typescript-migration branch
  2. npm link
  3. cio

running node dist/index.js after the build is also throwing the error.

I'm thinking it has something to do with the module type and/or compile target for TS. but haven't had a chance to really dig in and research it

@drewbrokke
Copy link
Collaborator

@jwu910 see #535.

drewbrokke and others added 13 commits July 12, 2023 22:15
@drewbrokke @jwu910
esm: sets package type to module
e4c14e1 
These commits (esm:) convert the project to support ESM following this guide:
https://gist.github.com/sindresorhus/a39789f98801d908bbc7ff3ecc99d99c
@drewbrokke @jwu910
esm: uses exports field instead of main
750ad39
@drewbrokke @jwu910
esm: requires node 16
6bdf847
@drewbrokke @jwu910
esm: uses recommended module target in tsconfig.json
89faf59
@drewbrokke @jwu910
esm: updates dependencies to ESM-only versions
56ae548
@drewbrokke @jwu910
esm: fixes new compile errors
a39b3de
@drewbrokke @jwu910
esm: partial revert of tsconfig.json
8cf97fe
@drewbrokke @jwu910
esm: moves all config logic to a dedicated file so we can remove the …
363479a 
…calls to require() and __dirname
@drewbrokke @jwu910
esm: namespaces node lib import
0172c0a
@drewbrokke @jwu910
esm: removes console.log
489a71f
@drewbrokke @jwu910
chore: adds .vscode dir to .gitignore
266812b
@drewbrokke @jwu910
chore: updates cz-conventional-changelog dependency
3c4e6a4
@jwu910
chore: bump nvmrc version
e8672a3
@jwu910
Copy link
Owner Author

jwu910 commented Jul 13, 2023

@jwu910 see #535.

I think we're good to merge 😂 if you get a minute tomorrow, would you be able to take another pass over the changes? not sure what else we need to do, either than maybe config clean up from some of the templated files. maybe some doc updates before any release, too

drewbrokke
drewbrokke approved these changes Jul 13, 2023
View reviewed changes
Copy link
Collaborator

@drewbrokke drewbrokke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM @jwu910 👍

Did you use tooling to accomplish 41cc6e1? If so, is it integrated into an npm script?

@jwu910
Copy link
Owner Author

jwu910 commented Jul 13, 2023

Lol @drewbrokke just vscode extension Document This, i haven't found a way to automate this based off type annotations yet though, I know there are tools that can generate docs from jsdoc strings, just wanted to keep it uniform where i could

@jwu910 jwu910 merged commit be1a258 into main Jul 13, 2023
4 checks passed
@jwu910 jwu910 deleted the typescript-migration branch July 13, 2023 14:53
This was referenced Jul 13, 2023
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@drewbrokke drewbrokke drewbrokke approved these changes

@kienD kienD Awaiting requested review from kienD kienD is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Feat: Upgrade to TS ➕ 🦖
4 participants
@jwu910 @drewbrokke @a-camarillo @Jimmy-Vu