Skip to content

refactor: Add TypeScript support #929

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

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

refactor: Add TypeScript support #929

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
1d61486
feat(ts): add TypeScript support, configuration, and missing type def…
fabiovincenzi Feb 17, 2025
c24ddf0
chore(ts): add typescript and vite to ignored dependencies
fabiovincenzi Mar 10, 2025
ee741e0
chore: remove lodash dependency
fabiovincenzi Mar 10, 2025
e316a95
feat(tsx): switch from ts-node to tsx and update dependencies
fabiovincenzi Mar 10, 2025
bdffb67
Merge branch 'main' into typescript-setup
fabiovincenzi Mar 20, 2025
5211d0e
Merge branch 'main' into typescript-setup
JamieSlome Mar 24, 2025
773754e
Merge branch 'main' into typescript-setup
fabiovincenzi Apr 2, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 4 additions & 5 deletions .github/workflows/unused-dependencies.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
name: 'Unused Dependencies'
on: [pull_request]

permissions:
contents: read
permissions:
contents: read

jobs:
unused-dependecies:
Expand All @@ -20,11 +20,10 @@ jobs:
with:
node-version: '18.x'
- name: 'Run depcheck'
run: |
npx depcheck --skip-missing --ignores="@babel/*,@commitlint/*,eslint,eslint-*,husky,mocha,concurrently,nyc,prettier"
run: |
npx depcheck --skip-missing --ignores="tsx,@babel/*,@commitlint/*,eslint,eslint-*,husky,mocha,concurrently,nyc,prettier,typescript,vite-tsconfig-paths"
echo $?
if [[ $? == 1 ]]; then
echo "Unused dependencies or devDependencies found"
exit 1
fi

27 changes: 15 additions & 12 deletions index.js → index.ts
View file
Open in desktop
Copy link
Contributor

@jescalada jescalada Mar 20, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

@JamieSlome @06kellyjac I was wondering if we want to continue refactoring the project into ESM or just keep the CommonJS for now.

I've tried refactoring various modules to TS and ESM in subsequent PRs. However, integration with Mocha tests and some ESM-only libraries (such as the load-plugin library in /src/plugin.js) causes many headaches with failing tests. I think converting into TS but keeping imports in CommonJS would be the least time-consuming.

On top of that, despite ESM being the "latest and greatest" way of doing things, there's a lot of resistance in the JS ecosystem because of integration problems with other libraries. Node 22 fixes these compatibility issues, but I'm unsure if forcing git-proxy users to use Node 22 is acceptable.

TL;DR: Should we convert to ESM and bump Node to 22 (to fix compatibility issues in test runs), or keep CommonJS and only refactor modules into TS?

I'd appreciate your thoughts on this! 😃

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for the input. I do enjoy a good upgrade but since 20LTS has security support for 1 more year and 18LTS still has 1 month it'd probably be sensible to stick to commonjs for the time being.

If we're building out javascript from our typescript for the published NPM package I'm thinking sticking with commonjs shouldn't be much of a problem for us.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you!

Original file line number Diff line number Diff line change
@@ -1,30 +1,37 @@
#!/usr/bin/env node
#!/usr/bin/env tsx
/* eslint-disable max-len */
const argv = require('yargs/yargs')(process.argv.slice(2))
import yargs from 'yargs';
import { hideBin } from 'yargs/helpers';
import * as fs from 'fs';
import config from './src/config/file';
import proxy from './src/proxy';
import service from './src/service';

const argv = yargs(hideBin(process.argv))
.usage('Usage: $0 [options]')
.options({
validate: {
description:
'Check the proxy.config.json file in the current working directory for validation errors.',
required: false,
alias: 'v',
type: 'boolean',
},
config: {
description: 'Path to custom git-proxy configuration file.',
default: 'proxy.config.json',
required: false,
alias: 'c',
type: 'string',
},
})
.strict().argv;
.strict()
.parseSync();

const config = require('./src/config/file');
config.configFile = argv.c ? argv.c : undefined;

if (argv.v) {
const fs = require('fs');

if (!fs.existsSync(config.configFile)) {
if (!fs.existsSync(config.configFile as string)) {
console.error(
`Config file ${config.configFile} doesn't exist, nothing to validate! Did you forget -c/--config?`,
);
Expand All @@ -38,11 +45,7 @@ if (argv.v) {

config.validate();

const proxy = require('./src/proxy');
const service = require('./src/service');

proxy.start();
service.start();

module.exports.proxy = proxy;
module.exports.service = service;
export { proxy, service };
Loading
Loading