Skip to content

Conversation

@gonfunko
Copy link
Contributor

The basics

The details

Resolves

Fixes #8775

Proposed Changes

This PR removes JSDom as a normal dependency, and adds the latest version as a peer and dev dependency. It also uses the browser field in package.json to indicate to bundlers and the like that it should be omitted for builds targeting a browser, rather than Node-based, deployment environment.

@gonfunko gonfunko requested a review from a team as a code owner January 22, 2026 00:00
@gonfunko gonfunko requested review from BenHenning and removed request for a team January 22, 2026 00:00
@github-actions github-actions bot added the PR: chore General chores (dependencies, typos, etc) label Jan 22, 2026
Copy link
Collaborator

@BenHenning BenHenning left a comment

Choose a reason for hiding this comment

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

Thanks @gonfunko! The change looks reasonable, though perhaps we may need to drop the version upgrade for now. Left one comment on that per the failing CI.

Edit: Also, it's very cool that the new package.json now implies Blockly has zero dependencies (though it's hard to be sure without scanning the package lock file).

"gulp-sourcemaps": "^3.0.0",
"gulp-umd": "^2.0.0",
"http-server": "^14.0.0",
"jsdom": "27.4.0",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Gemini thinks the upgraded version here is likely causing the build failures since this was part of the CI logs:

2026-01-22T00:00:56.5787404Z npm warn EBADENGINE Unsupported engine {
2026-01-22T00:00:56.5787981Z npm warn EBADENGINE   package: 'jsdom@27.4.0',
2026-01-22T00:00:56.5788484Z npm warn EBADENGINE   required: { node: '^20.19.0 || ^22.12.0 || >=24.0.0' },
2026-01-22T00:00:56.5788980Z npm warn EBADENGINE   current: { node: 'v18.20.8', npm: '10.8.2' }
2026-01-22T00:00:56.5789308Z npm warn EBADENGINE }

(I was curious but didn't actually want to debug).

Can we avoid the version upgrade for now?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think we actually just want to drop the old Node versions for v13, but we should have a team chat about how aggressive to be about that.

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

Labels

PR: chore General chores (dependencies, typos, etc)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants