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

Relax not-needed version condition #448

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion packages/definitions-parser/src/check-parse-results.ts
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ async function checkNpm(
log(" yarn not-needed " + yarnargs.join(" "));
log(` git add --all && git commit -m "${name}: Provides its own types" && git push -u origin not-needed-${name}`);
log(` And comment PR: This will deprecate \`@types/${name}\` in favor of just \`${name}\`. CC ${contributorUrls}`);
if (semver.gt(`${major}.${minor}.0`, firstTypedVersion)) {
if (semver.gte(`${major}.${minor}.0`, firstTypedVersion)) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Make the condition slightly more correct, same as

semver.gt(unneeded.version, latestTypings),
and
if (semver.lte(notNeeded, latest)) {
.

log(" WARNING: our version is greater!");
}
if (dependedOn.has(name)) {
Expand Down
20 changes: 6 additions & 14 deletions packages/definitions-parser/src/git.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import assert from "assert";
import { sourceBranch } from "./lib/settings";
import {
PackageId,
Expand All @@ -20,7 +19,6 @@ import {
cacheDir,
} from "@definitelytyped/utils";
import * as pacote from "pacote";
import * as semver from "semver";
import { getAffectedPackages } from "./get-affected-packages";

export interface GitDiff {
Expand Down Expand Up @@ -127,10 +125,14 @@ export async function getAffectedPackagesFromDiff(

/**
* 1. libraryName must exist on npm (SKIPPED and preferably/optionally have been the libraryName in just-deleted header)
* 2. asOfVersion must be newer than `@types/name@latest` on npm
* 3. `name@asOfVersion` must exist on npm
* 2. `name@asOfVersion` must exist on npm
*/
export async function checkNotNeededPackage(unneeded: NotNeededPackage) {
await pacote.manifest(unneeded.fullNpmName, { cache: cacheDir }).catch((reason) => {
throw reason.code === "E404"
? new Error(`Unexpected error: @types package not found for ${unneeded.fullNpmName}`, { cause: reason })
: reason;
}); // eg @types/babel__parser
await pacote.manifest(`${unneeded.libraryName}@${unneeded.version}`, { cache: cacheDir }).catch((reason) => {
throw reason.code === "E404"
? new Error(
Expand All @@ -145,16 +147,6 @@ Unneeded packages have to be replaced with a package on npm.`,
})
: reason;
}); // eg @babel/parser
const typings = await pacote.manifest(unneeded.fullNpmName, { cache: cacheDir }).catch((reason) => {
throw reason.code === "E404"
? new Error(`Unexpected error: @types package not found for ${unneeded.fullNpmName}`, { cause: reason })
: reason;
}); // eg @types/babel__parser
assert(
semver.gt(unneeded.version, typings.version),
`The specified version ${unneeded.version} of ${unneeded.libraryName} must be newer than the version
it is supposed to replace, ${typings.version} of ${unneeded.fullNpmName}.`
);
}

/**
Expand Down
30 changes: 13 additions & 17 deletions packages/definitions-parser/test/git.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,32 +96,28 @@ const nonexistentReplacementPackage = new NotNeededPackage("jest", "nonexistent"
const nonexistentTypesPackage = new NotNeededPackage("nonexistent", "jest", "100.0.0");

testo({
missingSource() {
return expect(checkNotNeededPackage(nonexistentReplacementPackage)).rejects.toThrow(
"The entry for @types/jest in notNeededPackages.json"
);
},
missingTypings() {
nonexistentTypesPackage() {
return expect(checkNotNeededPackage(nonexistentTypesPackage)).rejects.toThrow(
"@types package not found for @types/nonexistent"
);
},
deprecatedSameVersion() {
return expect(checkNotNeededPackage(sameVersion)).rejects
.toThrow(`The specified version 50.0.0 of jest must be newer than the version
it is supposed to replace, 50.0.0 of @types/jest.`);
},
deprecatedOlderVersion() {
return expect(checkNotNeededPackage(olderReplacement)).rejects
.toThrow(`The specified version 4.0.0 of jest must be newer than the version
it is supposed to replace, 50.0.0 of @types/jest.`);
nonexistentReplacementPackage() {
return expect(checkNotNeededPackage(nonexistentReplacementPackage)).rejects.toThrow(
"The entry for @types/jest in notNeededPackages.json"
);
},
missingNpmVersion() {
nonexistentReplacementVersion() {
return expect(checkNotNeededPackage(nonexistentReplacementVersion)).rejects.toThrow(
"The specified version 999.0.0 of jest is not on npm."
);
},
ok() {
newerReplacement() {
return checkNotNeededPackage(newerReplacement);
},
olderReplacement() {
return checkNotNeededPackage(olderReplacement);
},
sameVersion() {
return checkNotNeededPackage(sameVersion);
},
});