Address PR feedback

cryptodev-2s · cryptodev-2s · commit 7c90c8339e14 · 2025-03-13T21:35:05.000+01:00
diff --git a/src/release-specification.test.ts b/src/release-specification.test.ts
@@ -657,7 +657,7 @@ ${releaseSpecificationPath}
       });
     });
 
-    it('throws if there are any packages in the release with a major version bump using the word "major", but any of their dependents defined as "peerDependencies" have no changes since their latest release and are not listed in the release', async () => {
+    it('throws if there are any packages in the release with a major version bump using the word "major", but any of their dependents defined as "peerDependencies" are not listed in the release, even if they have no changes', async () => {
       await withSandbox(async (sandbox) => {
         const project = buildMockProject({
           workspacePackages: {
@@ -767,7 +767,7 @@ ${releaseSpecificationPath}
       });
     });
 
-    it('throws if there are any packages in the release with a major version bump using a literal version, but any of their dependents defined as "peerDependencies" have no changes since their latest release and are not listed in the release', async () => {
+    it('throws if there are any packages in the release with a major version bump using a literal version, but any of their dependents defined as "peerDependencies" are not listed in the release, even if they have no changes', async () => {
       await withSandbox(async (sandbox) => {
         const project = buildMockProject({
           workspacePackages: {
@@ -878,7 +878,7 @@ ${releaseSpecificationPath}
       });
     });
 
-    it('throws if there are any packages in the release with a major version bump using the word "major", but their dependents via "peerDependencies" have no changes since their latest release and have their version specified as null in the release spec', async () => {
+    it('throws if there are any packages in the release with a major version bump using the word "major", but their dependents via "peerDependencies" have their version specified as null in the release spec, even if they have no changes', async () => {
       await withSandbox(async (sandbox) => {
         const project = buildMockProject({
           workspacePackages: {
@@ -990,7 +990,7 @@ ${releaseSpecificationPath}
       });
     });
 
-    it('throws if there are any packages in the release with a major version bump using a literal version, but their dependents via "peerDependencies" have no changes since their latest release and have their version specified as null in the release spec', async () => {
+    it('throws if there are any packages in the release with a major version bump using a literal version, but their dependents via "peerDependencies" have their version specified as null in the release spec, even if they have no changes', async () => {
       await withSandbox(async (sandbox) => {
         const project = buildMockProject({
           workspacePackages: {
diff --git a/src/ui/App.tsx b/src/ui/App.tsx
@@ -13,7 +13,7 @@ const setsAreEqual = (a: Set<string>, b: Set<string>) => {
 };
 
 type SubmitButtonProps = {
-  selections: Record<string, string>;
+  releaseSelections: Record<string, string>;
   packageDependencyErrors: Record<
     string,
     { missingDependentNames: string[]; missingDependencies: string[] }
@@ -22,14 +22,16 @@ type SubmitButtonProps = {
 };
 
 function SubmitButton({
-  selections,
+  releaseSelections,
   packageDependencyErrors,
   onSubmit,
 }: SubmitButtonProps) {
   const isDisabled =
-    Object.keys(selections).length === 0 ||
+    Object.keys(releaseSelections).length === 0 ||
     Object.keys(packageDependencyErrors).length > 0 ||
-    Object.values(selections).every((value) => value === 'intentionally-skip');
+    Object.values(releaseSelections).every(
+      (value) => value === 'intentionally-skip',
+    );
 
   return (
     <button
@@ -48,7 +50,9 @@ function SubmitButton({
 
 function App() {
   const [packages, setPackages] = useState<Package[]>([]);
-  const [selections, setSelections] = useState<Record<string, string>>({});
+  const [releaseSelections, setReleaseSelections] = useState<
+    Record<string, string>
+  >({});
   const [isSubmitting, setIsSubmitting] = useState(false);
   const [error, setError] = useState<string | null>(null);
   const [changelogs, setChangelogs] = useState<Record<string, string>>({});
@@ -76,7 +80,7 @@ function App() {
   const previousPackages = useRef<Set<string>>(new Set());
 
   useEffect(() => {
-    const majorBumps = Object.entries(selections)
+    const majorBumps = Object.entries(releaseSelections)
       .filter(([_, type]) => type === 'major')
       .map(([pkgName]) => pkgName);
 
@@ -90,9 +94,9 @@ function App() {
       .then((data: Package[]) => {
         const newPackageNames = new Set(data.map((pkg) => pkg.name));
 
-        // Only clean up selections if the package list actually changed
+        // Only clean up releaseSelections if the package list actually changed
         if (!setsAreEqual(previousPackages.current, newPackageNames)) {
-          setSelections((prev) =>
+          setReleaseSelections((prev) =>
             Object.fromEntries(
               Object.entries(prev).filter(([pkgName]) =>
                 newPackageNames.has(pkgName),
@@ -111,7 +115,7 @@ function App() {
         setError(err.message);
         console.error('Error fetching packages:', err);
       });
-  }, [selections]);
+  }, [releaseSelections]);
 
   const checkDependencies = async (selectionData: Record<string, string>) => {
     if (Object.keys(selectionData).length === 0) return;
@@ -144,11 +148,11 @@ function App() {
 
   useEffect(() => {
     const timeoutId = setTimeout(() => {
-      void checkDependencies(selections);
+      void checkDependencies(releaseSelections);
     }, 500);
 
     return () => clearTimeout(timeoutId);
-  }, [selections]);
+  }, [releaseSelections]);
 
   const handleCustomVersionChange = (packageName: string, version: string) => {
     try {
@@ -178,7 +182,7 @@ function App() {
         return rest;
       });
 
-      setSelections((prev) => ({
+      setReleaseSelections((prev) => ({
         ...prev,
         [packageName]: version,
       }));
@@ -195,14 +199,14 @@ function App() {
     value: ReleaseType | '',
   ): void => {
     if (value === '') {
-      const { [packageName]: _, ...rest } = selections;
-      setSelections(rest);
+      const { [packageName]: _, ...rest } = releaseSelections;
+      setReleaseSelections(rest);
 
       const { [packageName]: __, ...remainingErrors } = packageDependencyErrors;
       setPackageDependencyErrors(remainingErrors);
     } else {
-      setSelections({
-        ...selections,
+      setReleaseSelections({
+        ...releaseSelections,
         [packageName]: value,
       });
     }
@@ -214,7 +218,7 @@ function App() {
       const response = await fetch('/api/release', {
         method: 'POST',
         headers: { 'Content-Type': 'application/json' },
-        body: JSON.stringify(selections),
+        body: JSON.stringify(releaseSelections),
       });
 
       const data: {
@@ -250,8 +254,8 @@ function App() {
       const errorMessage =
         err instanceof Error ? err.message : 'An error occurred';
       setError(errorMessage);
-      console.error('Error submitting selections:', err);
-      alert('Failed to submit selections. Please try again.');
+      console.error('Error submitting releaseSelections:', err);
+      alert('Failed to submit releaseSelections. Please try again.');
     } finally {
       setIsSubmitting(false);
     }
@@ -276,11 +280,11 @@ function App() {
   };
 
   const handleBulkAction = (action: ReleaseType) => {
-    const newSelections = { ...selections };
+    const newReleaseSelections = { ...releaseSelections };
     selectedPackages.forEach((packageName) => {
-      newSelections[packageName] = action;
+      newReleaseSelections[packageName] = action;
     });
-    setSelections(newSelections);
+    setReleaseSelections(newReleaseSelections);
     setSelectedPackages(new Set());
     setShowCheckboxes(true);
   };
@@ -389,7 +393,7 @@ function App() {
           <PackageItem
             key={pkg.name}
             pkg={pkg}
-            selections={selections}
+            releaseSelections={releaseSelections}
             versionErrors={versionErrors}
             packageDependencyErrors={packageDependencyErrors}
             loadingChangelogs={loadingChangelogs}
@@ -399,7 +403,7 @@ function App() {
             onSelectionChange={handleSelectionChange}
             onCustomVersionChange={handleCustomVersionChange}
             onFetchChangelog={fetchChangelog}
-            setSelections={setSelections}
+            setReleaseSelections={setReleaseSelections}
             setChangelogs={setChangelogs}
             onToggleSelect={() => togglePackageSelection(pkg.name)}
           />
@@ -408,7 +412,7 @@ function App() {
 
       {packages.length > 0 && (
         <SubmitButton
-          selections={selections}
+          releaseSelections={releaseSelections}
           packageDependencyErrors={packageDependencyErrors}
           onSubmit={handleSubmit}
         />
diff --git a/src/ui/PackageItem.tsx b/src/ui/PackageItem.tsx
@@ -6,7 +6,7 @@ import { Package, ReleaseType } from './types.js';
 
 type PackageItemProps = {
   pkg: Package;
-  selections: Record<string, string>;
+  releaseSelections: Record<string, string>;
   versionErrors: Record<string, string>;
   packageDependencyErrors: Record<
     string,
@@ -22,14 +22,16 @@ type PackageItemProps = {
   onSelectionChange: (packageName: string, value: ReleaseType | '') => void;
   onCustomVersionChange: (packageName: string, version: string) => void;
   onFetchChangelog: (packageName: string) => Promise<void>;
-  setSelections: React.Dispatch<React.SetStateAction<Record<string, string>>>;
+  setReleaseSelections: React.Dispatch<
+    React.SetStateAction<Record<string, string>>
+  >;
   setChangelogs: React.Dispatch<React.SetStateAction<Record<string, string>>>;
   onToggleSelect: () => void;
 };
 
 export function PackageItem({
   pkg,
-  selections,
+  releaseSelections,
   versionErrors,
   packageDependencyErrors,
   loadingChangelogs,
@@ -39,7 +41,7 @@ export function PackageItem({
   onSelectionChange,
   onCustomVersionChange,
   onFetchChangelog,
-  setSelections,
+  setReleaseSelections,
   setChangelogs,
   onToggleSelect,
 }: PackageItemProps) {
@@ -48,7 +50,8 @@ export function PackageItem({
       key={pkg.name}
       id={`package-${pkg.name}`}
       className={`border p-4 rounded-lg ${
-        selections[pkg.name] && selections[pkg.name] !== 'intentionally-skip'
+        releaseSelections[pkg.name] &&
+        releaseSelections[pkg.name] !== 'intentionally-skip'
           ? 'border-2'
           : 'border-gray-200'
       } ${
@@ -76,17 +79,19 @@ export function PackageItem({
           <div className="flex items-center justify-between">
             <div>
               <p className="text-gray-600">Current version: {pkg.version}</p>
-              {selections[pkg.name] &&
-                selections[pkg.name] !== 'intentionally-skip' &&
-                selections[pkg.name] !== 'custom' &&
+              {releaseSelections[pkg.name] &&
+                releaseSelections[pkg.name] !== 'intentionally-skip' &&
+                releaseSelections[pkg.name] !== 'custom' &&
                 !versionErrors[pkg.name] && (
                   <p className="text-yellow-700">
                     New version:{' '}
-                    {!['patch', 'minor', 'major'].includes(selections[pkg.name])
-                      ? selections[pkg.name]
+                    {!['patch', 'minor', 'major'].includes(
+                      releaseSelections[pkg.name],
+                    )
+                      ? releaseSelections[pkg.name]
                       : new SemVer(pkg.version)
                           .inc(
-                            selections[pkg.name] as Exclude<
+                            releaseSelections[pkg.name] as Exclude<
                               ReleaseType,
                               'intentionally-skip' | 'custom' | string
                             >,
@@ -102,7 +107,7 @@ export function PackageItem({
             </div>
             <VersionSelector
               packageName={pkg.name}
-              selection={selections[pkg.name]}
+              selection={releaseSelections[pkg.name]}
               onSelectionChange={onSelectionChange}
               onCustomVersionChange={onCustomVersionChange}
               onFetchChangelog={onFetchChangelog}
@@ -120,7 +125,7 @@ export function PackageItem({
               <DependencyErrorSection
                 title="Missing Dependencies"
                 items={packageDependencyErrors[pkg.name].missingDependencies}
-                setSelections={setSelections}
+                setSelections={setReleaseSelections}
                 description={`The following packages are dependencies or peer dependencies of ${pkg.name}. Because they may have introduced new changes that ${pkg.name} is now using, you need to verify whether to include them in the release.
 
 To do this, look at the change history for each package and compare it with the change history for ${pkg.name}. If ${pkg.name} uses any new changes from a package, then you need to include it by bumping its version. If you have confirmed that the changes to a package do not affect ${pkg.name}, you may omit it from the release by choosing "Skip" instead.`}
@@ -134,7 +139,7 @@ To do this, look at the change history for each package and compare it with the
                   items={
                     packageDependencyErrors[pkg.name].missingDependentNames
                   }
-                  setSelections={setSelections}
+                  setSelections={setReleaseSelections}
                   description={`Because ${pkg.name} is being released with a new major version, to prevent peer dependency warnings in consuming projects, all of the following packages which list ${pkg.name} as a peer dependency need to be included in the release. Please choose new versions for these packages. If for some reason you feel it is safe to omit a package you may choose "Skip".`}
                 />
               </div>
