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! #888

Merged
merged 66 commits into from
Nov 4, 2021
Merged

Typescript! #888

merged 66 commits into from
Nov 4, 2021

Conversation

raineorshine
Copy link
Owner

@raineorshine raineorshine commented May 2, 2021 •
edited

  • tsconfig
  • source maps
  • typescript-eslint
  • mocha integration
  • Split out getPeerDependencies
  • Split out version manager functions
  • Split out index functions
  • Convert npm to typescript
  • Convert yarn to typescript
  • Remove module.exports syntax
  • Remove vm from public API
  • Branded version types (optional)
    • Version, VersionRaw, VersionSpec
    • Also Github urls, npm urls, etc
  • Convert tests to typescript (optional)
    • A lot of tests!!! Can be done incrementally

@raineorshine raineorshine requested a review from XhmikosR May 2, 2021 21:43
@raineorshine
Copy link
Owner Author

@XhmikosR Do you have experience with Typescript? Would love to have someone else check over my setup. I haven't gotten very far with the actual conversion but it's compiling and tests are passing at least.

XhmikosR
XhmikosR reviewed May 3, 2021
View reviewed changes
package.json
@@ -30,21 +30,23 @@
"raine"
]
},
"main": "./lib",
"main": "./build/src",
"scripts": {
"build": "node scripts/build.js",
"lint": "cross-env FORCE_COLOR=1 npm-run-all --parallel --aggregate-output lint:*",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Unrelated but forgot to mention this the other day: why do we have FORCE_COLOR=1 here? If it's for CI, this is now fixed there and locally it shouldn't be needed. It's one less devDependency :)

Copy link
Owner Author

Choose a reason for hiding this comment

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

It's not just for CI. npm-run-all appears to have some known limitations with coloring. If I remove FORCE_COLOR=1 and run it locally it loses color output.

@XhmikosR
Copy link
Collaborator

XhmikosR commented May 3, 2021

I'm not familiar with TypeScript either just a couple of quick thoughts:

  1. make sure linting works properly for TS files and exclude the build folder
  2. make sure tsconfig.json has the proper target for the supported Node.js versions; no point in targeting Node.js 6 or 8 for example
  3. I'd switch to using xo for linting, or something else that does cover TS linting better

XhmikosR
XhmikosR reviewed May 3, 2021
View reviewed changes
.github/workflows/test.yml Outdated Show resolved Hide resolved
@raineorshine
Copy link
Owner Author

  1. make sure linting works properly for TS files and exclude the build folder

Yup, I still need to do that. I have a @typescript-eslint setup from another project I can use. I like the additional control of eslint.

2. make sure tsconfig.json has the proper target for the supported Node.js versions; no point in targeting Node.js 6 or 8 for example

Good call. I just switched to "es2018" which targets node v10 (and soon can bump to "es2019" when we remove v10 support).

@raineorshine raineorshine force-pushed the typescript branch 8 times, most recently from 43d3091 to 4acd967 Compare May 14, 2021 16:27
@raineorshine raineorshine force-pushed the typescript branch 7 times, most recently from a60b8f9 to c21da2c Compare May 30, 2021 13:01
@raineorshine raineorshine mentioned this pull request Sep 7, 2021
Open
3 tasks
@raineorshine
Convert > to >=. Fixes #957.
f773263
@raineorshine
Do not upgrade peerDependencies by default. Closes #951.
e70bd45
@raineorshine
Bump patch and minor dependencies
2c1c7be 
 prompts                 ^2.4.1  →    ^2.4.2
 eslint-plugin-import   ^2.24.2  →   ^2.25.2
 eslint-plugin-jsdoc    ^36.0.8  →   ^36.1.1
 markdownlint-cli       ^0.28.1  →   ^0.29.0
 yarn                  ^1.22.11  →  ^1.22.17
@raineorshine
Add typescript. Move ./lib to ./src
9af0435
@raineorshine
Fix src paths.
a03d961
@raineorshine
Add source-map-support. Fix tests.
462699d
@raineorshine
Rename get-ncu-rc to getNcuRc.
f6318bf
@raineorshine
Convert filter.test.js to Typescript.
0116bf8
@raineorshine
Move initOptions to /src/lib
744a34b
@raineorshine
export default run and move Typescript tests to /src/test.
7ca73ba
@raineorshine
Move runLocal and runLocal to /src/lib
7c6622c
@raineorshine
Move filterAndReject to /src/lib
9ba4242
@raineorshine
Use semver.satisfies instead of isSatisfied alias.
af0ce14
@raineorshine
Dissolve versionmanager
71ba819 
Move to /src/lib: getCurrentDependencies, getInstalledPackages, upgradePackageData
@raineorshine
Remove mock-require. Fix spawn yarn guard.
6f640a1
@raineorshine
Convert getRepoUrl to Typescript.
efa63ef
@raineorshine
Convert cli-options to Typescript.
a38d300
@raineorshine
Convert doctor to Typescript.
87ee36c
@raineorshine
version-util: Remove module.exports syntax.
be5ada2
@raineorshine
Convert cli to Typescript.
6f24779
@raineorshine
Bump dependencies
36c0d3d 
 @types/lodash                     ^4.14.168  →  ^4.14.170
 @types/node                         ^15.0.1  →    ^15.6.1
 @types/prompts                      ^2.0.11  →    ^2.0.12
 @types/semver                        ^7.3.5  →     ^7.3.6
 @typescript-eslint/eslint-plugin    ^4.22.1  →    ^4.25.0
 @typescript-eslint/parser           ^4.22.1  →    ^4.25.0
 eslint                              ^7.26.0  →    ^7.27.0
 eslint-config-standard              ^16.0.2  →    ^16.0.3
 eslint-plugin-import                ^2.23.0  →    ^2.23.3
 eslint-plugin-jsdoc                 ^34.2.2  →    ^35.0.0
 ts-node                              ^9.1.1  →    ^10.0.0
 typescript                           ^4.2.4  →     ^4.3.2
@raineorshine
Use PATH in tests to use local yarn rather than NCU_TESTS. Add missin…
a5de794 
…g license fields.
@raineorshine
.ncurc comment
b2a4245
@raineorshine
Rename VersionDeclaration to VersionSpec.
17ba16a
@raineorshine
12.0.0-0
4b9e79f
@raineorshine
README
8e895fc
@raineorshine
lint
01dcc5c
@raineorshine
Fix typescript errors.
5c691db
@raineorshine
Remove explicit types field in package.json.
81e8633
@raineorshine
Bump node engine to v12
7783f8e
@raineorshine
Fix upgradeGithubUrl.
119206a
@raineorshine raineorshine merged commit f1d6a2d into main Nov 4, 2021
@raineorshine raineorshine deleted the typescript branch November 4, 2021 13:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@XhmikosR XhmikosR XhmikosR left review comments

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

Successfully merging this pull request may close these issues.

None yet
2 participants
@raineorshine @XhmikosR