Skip to content
Merged
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 README.md
Original file line number Diff line number Diff line change
Expand Up @@ -420,7 +420,7 @@ To upgrade yarn itself, download a new yarn release from
Note that when upgrading @patternfly packages, we've seen in the past that it can cause the JavaScript heap to run out of memory, or the bundle being too large if multiple versions of the same @patternfly package is pulled in. To increase efficiency, run the following after updating packages:

```
npx yarn-deduplicate --scopes @patternfly
yarn run dedupe-deps --scopes @patternfly
```

#### Supported Browsers
Expand Down
17 changes: 10 additions & 7 deletions frontend/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,11 @@
"integration-tests"
],
"scripts": {
"postinstall": "./scripts/check-patternfly-modules.sh && yarn prepare-husky",
"postinstall": "yarn ts-node ./scripts/check-patternfly-modules.ts && yarn prepare-husky",
"clean": "rm -rf ./public/dist",
"dev": "yarn clean && yarn generate && REACT_REFRESH=true NODE_OPTIONS=--max-old-space-size=4096 yarn ts-node ./node_modules/.bin/webpack serve --mode=development --progress",
"dev-once": "yarn clean && yarn generate && NODE_OPTIONS=--max-old-space-size=4096 yarn ts-node ./node_modules/.bin/webpack --mode=development",
"dedupe-deps": "yarn-deduplicate --strategy highest",
"build": "yarn clean && yarn generate && NODE_ENV=production NODE_OPTIONS=--max-old-space-size=4096 yarn ts-node ./node_modules/.bin/webpack --mode=production",
"check-cycles": "CHECK_CYCLES=true yarn dev-once",
"coverage": "jest --coverage .",
Expand Down Expand Up @@ -136,8 +137,7 @@
"@patternfly/react-catalog-view-extension": "^6.1.0",
"@patternfly/react-charts": "^8.2.2",
"@patternfly/react-code-editor": "^6.2.2",
"@patternfly/react-component-groups": "6.2.0-prerelease.10",
"@patternfly/react-console": "^6.0.0",
"@patternfly/react-component-groups": "^6.3.0",
"@patternfly/react-core": "^6.2.2",
"@patternfly/react-data-view": "^6.2.0",
"@patternfly/react-icons": "^6.2.2",
Expand Down Expand Up @@ -234,7 +234,7 @@
"@types/jasmine": "2.8.x",
"@types/jest": "22.x",
"@types/json-schema": "^7.0.7",
"@types/lodash-es": "4.17.x",
"@types/lodash-es": "^4.17.12",
"@types/node": "22.x",
"@types/prop-types": "15.5.6",
"@types/react": "17.x",
Expand All @@ -245,6 +245,8 @@
"@types/react-virtualized": "9.x",
"@types/semver": "^6.0.0",
"@types/showdown": "1.9.4",
"@types/yarnpkg__lockfile": "^1.1.9",
"@yarnpkg/lockfile": "^1.1.0",
"axe-core": "^4.10.2",
"babel-loader": "^8.2.1",
"browser-env": "3.x",
Expand Down Expand Up @@ -303,14 +305,15 @@
"webpack": "^5.75.0",
"webpack-bundle-analyzer": "4.10.2",
"webpack-cli": "^5.1.4",
"webpack-dev-server": "^5.1.0"
"webpack-dev-server": "^5.1.0",
"yarn-deduplicate": "^6.0.2"
},
"engines": {
"node": ">=22.x"
},
"resolutions": {
"@patternfly/react-component-groups": "6.2.0-prerelease.10",
"@patternfly/react-data-view": "^6.2.0",
"@patternfly/react-component-groups": "6.3.0",
"@types/lodash": "4.14.106",
"@types/react-router": "^5.1.20",
"@types/react-router-dom": "5.3.x",
"hosted-git-info": "^3.0.8",
Expand Down
88 changes: 0 additions & 88 deletions frontend/scripts/check-patternfly-modules.sh

This file was deleted.

123 changes: 123 additions & 0 deletions frontend/scripts/check-patternfly-modules.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,123 @@
/* eslint-disable no-console */
import * as lockfile from '@yarnpkg/lockfile';
import { readFileSync } from 'fs';
import { basename } from 'path';
import chalk from 'chalk';
import * as semver from 'semver';

console.log(`Checking ${chalk.yellow('yarn.lock')} for PatternFly module resolutions...`);

const lockFile = lockfile.parse(readFileSync('yarn.lock', 'utf8'));

if (lockFile.type !== 'success') {
console.error(`Failed to parse yarn.lock file: type is ${lockFile.type}`);
process.exit(1);
}

/** List of packages to check along with their required semver ranges */
const PKGS_TO_CHECK: Array<{ name: string; semver: string }> = [
{ name: '@patternfly-5/patternfly', semver: '5.x' },
{ name: '@patternfly/patternfly', semver: '6.x' },
{ name: '@patternfly/quickstarts', semver: '6.x' },
{ name: '@patternfly/react-catalog-view-extension', semver: '6.x' },
{ name: '@patternfly/react-charts', semver: '8.x' },
{ name: '@patternfly/react-code-editor', semver: '6.x' },
{ name: '@patternfly/react-component-groups', semver: '6.x' },
{ name: '@patternfly/react-core', semver: '6.x' },
{ name: '@patternfly/react-data-view', semver: '6.x' },
{ name: '@patternfly/react-icons', semver: '6.x' },
{ name: '@patternfly/react-log-viewer', semver: '6.x' },
{ name: '@patternfly/react-styles', semver: '6.x' },
{ name: '@patternfly/react-table', semver: '6.x' },
{ name: '@patternfly/react-templates', semver: '6.x' },
{ name: '@patternfly/react-tokens', semver: '6.x' },
{ name: '@patternfly/react-topology', semver: '6.x' },
{ name: '@patternfly/react-user-feedback', semver: '6.x' },
{ name: '@patternfly/react-virtualized-extension', semver: '6.x' },
];

const SCOPES_TO_CHECK = new Set(PKGS_TO_CHECK.map((pkg) => pkg.name.split('/')[0]));

/** Get the package name and version from a package key in yarn.lock */
const parsePackageName = (resolutionKey: string): string => {
// Precondition: All package names are not substrings of other package names
// e.g., we cannot have both `my-package` and `my-package-extended` in PKGS_TO_CHECK
const pkgName = PKGS_TO_CHECK.find((pkg) => resolutionKey.startsWith(pkg.name));
Copy link
Member

Choose a reason for hiding this comment

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

Not a problem with the current packages, but could we ever have a false positive here if one package name is the prefix of another like my-package and my-package-utils?

Copy link
Member Author

Choose a reason for hiding this comment

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

Probably..? I think we can fix this problem when we need to, it's hard to properly parse the versions of resolutionKey since there are a few edgecases like @patternfly-5/patternfly@npm:@patternfly/patternfly@5.4.2.


// Ensure that all packages within SCOPES_TO_CHECK are checked
if (!pkgName) {
throw new Error(
`Please update ${chalk.yellow(
basename(__filename),
)} to handle this PatternFly package: ${chalk.yellow(resolutionKey)}`,
);
}

return pkgName.name;
};

/** Map of PatternFly packages to their resolved versions in yarn.lock */
const patternflyModules = Object.entries(lockFile.object).reduce(
(acc, [resolutionKey, resolvedDependency]) => {
if (SCOPES_TO_CHECK.has(resolutionKey.split('/')[0])) {
const pkgName = parsePackageName(resolutionKey);
if (!acc.has(pkgName)) {
acc.set(pkgName, []);
}
acc.get(pkgName).push({ resolvedVersion: resolvedDependency.version });
}

return acc;
},
new Map<string, Array<{ resolvedVersion: string }>>(),
);
Comment on lines +60 to +73
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Scope filtering should use the parsed package name, not resolutionKey.split('/')
If keys are quoted / comma-delimited, resolutionKey.split('/')[0] may be "@patternfly (with a quote) and skip PatternFly entries entirely.

Suggested direction: compute const name = packageNameFromSelector(firstSelector(resolutionKey)), then check SCOPES_TO_CHECK.has(name.split('/')[0]) before aggregating.

🤖 Prompt for AI Agents
In frontend/scripts/check-patternfly-modules.ts around lines 58 to 71, the scope
check uses resolutionKey.split('/')[0] which can fail for quoted or
comma-delimited keys; instead derive the package selector first and compute the
parsed package name, e.g. const name =
packageNameFromSelector(firstSelector(resolutionKey)), then use
SCOPES_TO_CHECK.has(name.split('/')[0]) for the scope check before aggregating;
keep the existing parsePackageName(resolutionKey) usage for the map key and push
resolvedVersion as before, ensuring the scope check uses the parsed package name
rather than raw resolutionKey.split('/') output.


let hasResolutionErrors = false;

for (const pkg of PKGS_TO_CHECK) {
const resolvedVersions = Array.from(
new Set((patternflyModules.get(pkg.name) || []).map((v) => v.resolvedVersion)),
);

if (resolvedVersions.length === 0) {
console.error(`${chalk.red(pkg.name)} has no ${chalk.yellow(pkg.semver)} resolutions`);
hasResolutionErrors = true;
} else if (resolvedVersions.length === 1) {
const resolvedVersion = semver.coerce(resolvedVersions[0]);

if (!resolvedVersion) {
console.error(
`${chalk.red(pkg.name)} has a non-semver coercible resolved version: ${chalk.yellow(
resolvedVersions[0],
)}`,
);
hasResolutionErrors = true;
continue;
}

if (!semver.satisfies(resolvedVersion, pkg.semver)) {
console.error(
`${chalk.red(pkg.name)} has one resolution ${chalk.yellow(
resolvedVersions[0],
)} which does not satisfy ${chalk.yellow(pkg.semver)}`,
);
hasResolutionErrors = true;
} else {
console.log(
`${chalk.green(pkg.name)} has one ${chalk.yellow(pkg.semver)} resolution: ${
resolvedVersions[0]
}`,
);
}
} else {
console.error(`${chalk.red(pkg.name)} has multiple resolutions: ${resolvedVersions}`);
hasResolutionErrors = true;
}
}
Comment on lines +77 to +116
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Resolved dependency version may be missing; guard before semver logic
resolvedDependency.version is assumed present. If the lock entry is malformed/partial, you’ll end up with undefined in resolvedVersions and then hit the “non-semver coercible” path with a confusing message.

Consider filtering falsy versions at collection time (Line 65) or when building resolvedVersions.

🤖 Prompt for AI Agents
In frontend/scripts/check-patternfly-modules.ts around lines 75 to 114,
resolvedVersions may include undefined/null/empty values from malformed lock
entries which leads to confusing "non-semver coercible" errors; filter out
falsy/non-string versions when building resolvedVersions (e.g., remove
undefined/null/empty before creating the Set) and treat the case where filtering
leaves zero entries as "no resolutions" so semver.coerce is only called with
valid strings.


if (hasResolutionErrors) {
console.error(`Run ${chalk.yellow('yarn why <pkg-name>')} to inspect module resolution details`);
process.exit(1);
}

console.log(chalk.green('No issues detected'));
Loading