Hacktoberfest 2026: the issues maintainers tagged for October, open and beginner-friendly. Browse Hacktoberfest issues

dtslint-runner should not compare against master on DT PRs

Open
  • Is something wrong?
#897 0 comments 1 reaction 0 assignees View on GitHub

Maintainers usually reply within 1 day

Nobody has claimed this yet.

Assessment

Difficulty
4/5
Estimated time
3-5 days
Newbie friendliness
35/100
Issue type
Bug
Clarity
Mostly clear
Activity status
Stale
Tech stack
shell, typescript
Domain
ci-cd, tooling

Research direction

Locate dtslint-runner and its git diff logic, then compare it with the merge-base handling in scripts/pnpm-install.sh. Check how CI identifies pull requests and how merge commits expose HEAD^1. Done means PR runs compare against the PR's stable merge base rather than a newer origin/master, so unrelated packages are not reported as changed.

Written by the indexing model from the issue text.

Description

dtslint-runner diffs against master. If someone merges a PR at the exact same time as another PR's CI runs, they'll pull origin/master and may get the wrong thing. For example, in https://github.com/DefinitelyTyped/DefinitelyTyped/actions/runs/7452339628/job/20275379960?pr=68135:

Running: git rev-parse --verify master
Running: git fetch origin master

Running: git branch master FETCH_HEAD

Running: git diff master --name-status
M	types/matter-js/index.d.ts
M	types/slate-html-serializer/index.d.ts
M	types/slate-html-serializer/tsconfig.json
Testing 2 changed packages: Set(2) { 'matter-js', 'slate-html-serializer' }
Testing 0 dependent packages: Set(0) {}

dtslint-runner fetches master, but gets a newer master than the PR, so it believes that the PR modified matter-js, when it was actually another PR merged to master. I partially fixed this for pnpm install in https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/scripts/pnpm-install.sh; since PRs are run on merge commits in CI, HEAD^1 points to an already pulled / stable merge base (what the PR thinks master is at the time of the CI run; further runs will get newer merge bases so it's not out of date).

This then causes further failures because pnpm install is installing the "correct" dependency set, which does not include matter-js.

In CI, it's easy to tell that we're in a PR. But, I'm not totally sure of the right mechanism to check in dtslint-runner, but we should definitely do it.

Dominant language
TypeScript
Stars
422
Forks
238
Avg merge
1d 4h
Merged PRs (30d)
8

Getting set up

This project ships no dev container, Dockerfile or contributing guide, so setting up is up to you: start from its README, and see our first-contribution guide for the general steps.

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from microsoft/DefinitelyTyped-tools

All issues in microsoft/DefinitelyTyped-tools

Similar issues

More TypeScript issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.