Skip to content

fix: security bump versions #86

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

Jump to bottom
Open
GordonSmith wants to merge 1 commit into
base: candidate-1.10.1
Choose a base branch
from
Open

fix: security bump versions #86

GordonSmith wants to merge 1 commit into from

Conversation

@GordonSmith
Copy link
Member

@GordonSmith GordonSmith commented Jan 22, 2026

No description provided.

Copilot AI reviewed Jan 22, 2026
View reviewed changes
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This pull request performs a major version bump of dependencies and modernizes the build configuration to address security concerns. The changes update TypeScript compilation targets, module formats, and all @hpcc-js dependencies from version 2.x to 3.x.

Changes:

  • Updated TypeScript target from ES5 to ES2022 and module format from UMD to ES modules
  • Upgraded all @hpcc-js dependencies to version 3.x (major version bump)
  • Modernized build configuration (Rollup, TypeScript compiler options)
  • Updated CDN links in HTML to use ES module imports with import maps
  • Fixed strictNullChecks compatibility issues and formatting

Reviewed changes

Copilot reviewed 7 out of 9 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
report/tsconfig.json Updated TypeScript target to ES2022, enabled strict mode, simplified lib configuration
report/src/statChart.ts Added default initializer for _tickFormatter to satisfy strict null checks
report/src/index.ts Updated imports from WUInfo to WsWorkunits API, improved code formatting
report/runTest.ecl Changed test data file path from vehiclereexpanded to multi::person
report/rollup.config.js Migrated to ES modules, updated to use modern Node.js APIs (readFileSync with import.meta.url)
report/res/index.min.js Regenerated minified output to reflect ES module format
report/res/index.html Updated CDN versions to 3.x, implemented import maps, changed debug URL to localhost
report/package.json Bumped all dependencies to latest 3.x versions, added update scripts
report/package-lock.json Updated lock file to reflect new dependency versions

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

report/tsconfig.json
Comment on lines +4 to +5
"target": "es2022",
"module": "es2022",
Copy link

Copilot AI Jan 22, 2026

Choose a reason for hiding this comment

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

The target has been updated from "es5" to "es2022" and module from "umd" to "es2022". This is a significant change that may break compatibility with older browsers and Node.js versions. Ensure this aligns with your project's browser/runtime support requirements and that all consumers of this library support ES2022.

Suggested change
"target": "es2022",
"module": "es2022",
"target": "es5",
"module": "umd",

Copilot uses AI. Check for mistakes.
Copy link
Member Author

@GordonSmith GordonSmith Jan 22, 2026

Choose a reason for hiding this comment

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

Should be fine.

report/tsconfig.json
Comment on lines 13 to +15
"lib": [
"dom",
"es5",
"es2015.promise"
"es2022"
Copy link

Copilot AI Jan 22, 2026

Choose a reason for hiding this comment

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

The library list has been simplified from ["dom", "es5", "es2015.promise"] to just ["es2022"]. The "es2022" lib includes all ES2022 features. Verify that "dom" types are still available when needed, or add it back if DOM APIs are used in this project.

Copilot uses AI. Check for mistakes.
Copy link
Member Author

@GordonSmith GordonSmith Jan 22, 2026

Choose a reason for hiding this comment

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

DOM was not removed.

report/src/index.ts
Comment on lines +2 to +9
import { Result, Workunit, WsWorkunits } from "@hpcc-js/comms";
import { Grid } from "@hpcc-js/layout";
import { Html } from "@hpcc-js/other";
import { DockPanel } from "@hpcc-js/phosphor";
import { BreakdownTable, StyledTable } from "@hpcc-js/html";
import { StatChart } from "./statChart";

const knownProfileField = (sch: WUInfo.ECLSchemaItem): boolean => ["attribute", "given_attribute_type", "best_attribute_type", "rec_count", "fill_count", "fill_rate", "cardinality", "cardinality_breakdown", "modes", "min_length", "max_length", "ave_length", "popular_patterns", "rare_patterns", "is_numeric", "numeric_min", "numeric_max", "numeric_mean", "numeric_std_dev", "numeric_lower_quartile", "numeric_median", "numeric_upper_quartile", "numeric_correlations"].indexOf(sch.ColumnName) > 0;
const knownProfileField = (sch: WsWorkunits.ECLSchemaItem): boolean => ["attribute", "given_attribute_type", "best_attribute_type", "rec_count", "fill_count", "fill_rate", "cardinality", "cardinality_breakdown", "modes", "min_length", "max_length", "ave_length", "popular_patterns", "rare_patterns", "is_numeric", "numeric_min", "numeric_max", "numeric_mean", "numeric_std_dev", "numeric_lower_quartile", "numeric_median", "numeric_upper_quartile", "numeric_correlations"].indexOf(sch.ColumnName) > 0;
Copy link

Copilot AI Jan 22, 2026

Choose a reason for hiding this comment

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

The import has been changed from "WUInfo" to "WsWorkunits". This appears to be an API change in the @hpcc-js/comms package. Verify that all usages of WUInfo.ECLSchemaItem throughout the codebase have been updated to WsWorkunits.ECLSchemaItem.

Copilot uses AI. Check for mistakes.
Copy link
Member Author

@GordonSmith GordonSmith Jan 22, 2026

Choose a reason for hiding this comment

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

It has

report/rollup.config.js
output: {
file: pkg.main,
format: "umd",
format: "es",
Copy link

Copilot AI Jan 22, 2026

Choose a reason for hiding this comment

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

The rollup output format has been changed from "umd" to "es". This is a breaking change that will affect how the module can be consumed. UMD format supports multiple module systems (AMD, CommonJS, and global variables), while ES format only supports ES modules. Ensure all consumers of this library support ES modules, or consider maintaining backward compatibility.

Suggested change
format: "es",
format: "umd",

Copilot uses AI. Check for mistakes.
Copy link
Member Author

@GordonSmith GordonSmith Jan 22, 2026

Choose a reason for hiding this comment

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

The html was updated to accomidate this

report/res/index.html Outdated
var debugWuid = window.location.search.split("?")[1];
if (debugWuid) {
espUrl = "http://play.hpccsystems.com:8010/WsWorkunits/res/" + debugWuid + "/report/res/index.html";
espUrl = "http://localhost:8010/WsWorkunits/res/" + debugWuid + "/report/res/index.html";
Copy link

Copilot AI Jan 22, 2026

Choose a reason for hiding this comment

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

The debug URL has been changed from "play.hpccsystems.com" to "localhost". This appears to be a development-specific change that should not be committed to the main branch. Consider reverting this to the production URL or making it configurable via environment variables.

Suggested change
espUrl = "http://localhost:8010/WsWorkunits/res/" + debugWuid + "/report/res/index.html";
espUrl = "https://play.hpccsystems.com/WsWorkunits/res/" + debugWuid + "/report/res/index.html";

Copilot uses AI. Check for mistakes.
Copy link
Collaborator

@dcamper dcamper Jan 22, 2026

Choose a reason for hiding this comment

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

@GordonSmith Is this valid? Should the URL reference the play system?

Copy link
Member Author

@GordonSmith GordonSmith Jan 22, 2026

Choose a reason for hiding this comment

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

I will revert - but its only for developer testing (me and you)

@dcamper dcamper changed the base branch from master to candidate-1.10.1 January 22, 2026 13:00
dcamper
dcamper requested changes Jan 22, 2026
View reviewed changes
Copy link
Collaborator

@dcamper dcamper left a comment

Choose a reason for hiding this comment

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

One question before I approve.

Note that I changed the PR's base.

report/res/index.html Outdated
var debugWuid = window.location.search.split("?")[1];
if (debugWuid) {
espUrl = "http://play.hpccsystems.com:8010/WsWorkunits/res/" + debugWuid + "/report/res/index.html";
espUrl = "http://localhost:8010/WsWorkunits/res/" + debugWuid + "/report/res/index.html";
Copy link
Collaborator

@dcamper dcamper Jan 22, 2026

Choose a reason for hiding this comment

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

@GordonSmith Is this valid? Should the URL reference the play system?

@GordonSmith
fix: security bump versions
ec0e945 
Signed-off-by: Gordon Smith <GordonJSmith@gmail.com>
@GordonSmith GordonSmith requested a review from dcamper January 22, 2026 15:48
@GordonSmith
Copy link
Member Author

GordonSmith commented Jan 22, 2026 •
edited
Loading

I also updated the CDN Urls to be cleaner as per: https://www.jsdelivr.com/?docs=esm

dcamper
dcamper approved these changes Jan 22, 2026
View reviewed changes
Copy link
Collaborator

@dcamper dcamper left a comment

Choose a reason for hiding this comment

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

Looks good, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

@dcamper dcamper dcamper approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@GordonSmith @dcamper