feat(react-dnd): Fork debt: react-dnd (25 criticals) - untangle app.zenchef.com drag-and-drop

[Marian-zenchef]

Summary

https://zenchef.atlassian.net/browse/TS-1344

Rebases @zenchef/react-dnd-html5-backend forward to reduce the Dependabot criticals/highs without changing its v2-era API (still satisfies react-dnd@^7.7.0's manager interface, so no consumer call-site changes are needed).

• Bump lodash to ^4.17.21 (patched) and pin known-vulnerable transitive deps via root resolutions, then regenerate yarn.lock so those resolutions actually take effect.
• Give the package its own modern @babel/cli / @babel/preset-env build pipeline (babel.build.config.json), replacing the ancient root Babel 6 toolchain for this package's build.
• Fix a real syntax bug in HTML5Backend.js (getCurrentSourceNodeOptions) — a stray } left over from the shiftKey/ctrlKey modifier commit — that made the package fail to build from source on main (confirmed: @babel/core throws parsing it as-is today).
• Fix package identity: name was still the unscoped upstream react-dnd-html5-backend with repository.url pointing at react-dnd/react-dnd — publishing as-is would not have produced @zenchef/react-dnd-html5-backend. Now scoped correctly, with publishConfig.registry pointing at GitHub Packages.
• Rename prepublish → prepublishOnly — prepublish doesn't run on npm publish under npm ≥ 7, so a fresh-clone publish shipped a tarball with no lib/.
• Made the package fully self-contained: added local rimraf, dropped the npm-run-all/build:lib indirection (one build target, no longer needs parallelizing), and pointed every script at ./node_modules/.bin instead of the monorepo root. prepublishOnly is npm run clean && npm run build && npm test. Verified by removing the root node_modules entirely and re-running npm ci + npm publish --dry-run from just this directory — works standalone now.
• Regenerate packages/react-dnd-html5-backend/package-lock.json against the corrected package.json (previous one had @babel/* misfiled under dependencies and was missing react-dnd's transitive deps — npm ci failed). Dropped packages/react-dnd/package-lock.json and packages/dnd-core/package-lock.json — both unrelated to this PR (their package.json files are untouched) and the react-dnd one resolved dnd-core@2.6.0 from the public registry instead of this repo's own 2.5.4.
• Removed the second build toolchain: build:umd (root webpack 3 + Babel 6 .babelrc) produced dist/ReactDnDHTML5Backend.min.js, which nothing consumes (main is lib/index.js, no unpkg/browser field). Deleted build:umd and packages/react-dnd-html5-backend/webpack.config.js. Also cleaned up the now-stale references: README no longer advertises the UMD/dist build, .npmignore no longer lists the deleted webpack.config.js.
• Wired npm test to actually run test/HTML5Backend.spec.js — it previously ran clean && build only. Added package-local jest/babel-jest/jest-environment-jsdom devDependencies (the spec only imports via relative path, so it never needed root's jest/moduleNameMapper).
• Added real coverage for getCurrentSourceNodeOptions/getCurrentDropEffect — the exact function this PR repairs had zero tests, which is how the syntax error sat unnoticed on main. 6 new cases (modifier-key precedence, source-supplied dropEffect override). 9/9 tests pass.
• Bump package version to 2.5.6.

Root-level tooling (site/, examples/, and the devDependencies used only to build/serve the unused docs site) was intentionally left untouched — out of scope for this pass. Same for CI (.travis.yml is an empty stub, no .github/ workflows at all) — real gap, but a separate follow-up ticket alongside the root-tooling prune, not an expansion of this PR.

Dependabot impact (verified against the live alerts, not estimated): current main has 25 open critical + 94 open high alerts. Cross-referencing each alert's vulnerable-version range against the 18 resolutions: 24/25 criticals and 18/94 highs are closed by this PR. The remaining 1 critical (babel-traverse) and 76 highs all trace back to the same out-of-scope root Babel6/webpack3/docs-site toolchain.

Known pre-existing behavior worth knowing for the app-side check: if a drag source passes its own dropEffect via connectDragSource(node, options), that silently overrides the shift/ctrl/alt modifier keys (lodash.defaults semantics, unchanged by this PR). If the table-plan ever does that, shift/ctrl will no-op there — worth checking explicitly rather than assuming.

Test plan

• npm run build compiles cleanly, all output files pass node --check
• npm test runs the real jest suite — 9/9 pass, including new coverage for the exact function this PR fixes
• npm audit on the package: 0 vulnerabilities
• npm ci succeeds against the regenerated lockfile
• Package installs, builds, tests, and publishes (--dry-run) standalone, with the monorepo root's node_modules entirely removed
• Publish 2.5.6 to GitHub Packages and verify table-plan / reservation drag-and-drop on app.zenchef.com before merge — including the dropEffect-override case above

[olivier-zenchef]

Reviewed the full diff at 5255a869 — 4 hand-written files plus 4 lockfiles. I fetched the branch and verified the build, the publish behaviour and the lockfiles empirically rather than reading them only.

2 Critical, 4 Important, 3 Suggestions. The two Criticals both block the last unchecked test-plan box (publish 2.5.6 and verify on app.zenchef.com) — neither is about the code change itself, both are about the package never reaching the app in a usable form.

Important — no diff line to anchor to

lerna.json:3 — still "version": "2.5.4" while the package moves to 2.5.6. Lerna 2 in fixed mode treats lerna.json as the source of truth, and the branch that actually published (zenchef/shift-and-ctrl-keys) kept the two in step at 2.5.5/2.5.5. Worth syncing so the next lerna publish doesn't fight the manual bump.

What's verified good

• The HTML5Backend.js fix is byte-identical to what origin/zenchef/shift-and-ctrl-keys already shipped as 2.5.5 — this is main catching up to production, not a new behaviour change. Reviewers should size it accordingly.
• The new Babel 7 pipeline works end to end: babel src --out-dir lib --no-babelrc --config-file ./babel.build.config.json compiles all 10 files and every emitted file passes node --check. CJS interop and the exports.default shape are unchanged from the Babel 6 output.
• All 18 resolutions genuinely landed in yarn.lock — I checked each one (lodash 4.17.21, minimist 1.2.6, handlebars 4.7.9, elliptic 6.6.1, …). The regeneration was done properly, and it even collapsed a stale form-data@~2.1.1 branch.
• Leaving site/, examples/ and root build tooling alone was the right scope call.

Suggested order

1. Port the fork identity from zenchef/shift-and-ctrl-keys (name, repository.url, publishConfig).
2. Rename prepublish → prepublishOnly.
3. Drop the two unrelated lockfiles, regenerate the third.
4. Sync lerna.json.
5. Post the Dependabot before/after count, or soften the "clears the criticals" claim and ticket the rest.

[olivier-zenchef]

Round 2 — reviewed 5255a8690...a193b171e (the new commit), with the full main...a193b171e diff re-read since it's small. Verified in a clean worktree at a193b171e.

8 of the 9 open threads are verified fixed, and the Dependabot thread is properly answered — you pulled the live alert counts instead of rewording blind, and the description now says 24/25 criticals and 18/94 highs with the remainder attributed to a named out-of-scope toolchain. That's the right resolution. The lockfile is correct (npm ci succeeds from clean, 0 vulnerabilities, root entry has the right name/version and @babel/* properly under devDependencies, and npm install doesn't rewrite it so it won't churn under lerna bootstrap). npm publish --dry-run from a lib/-deleted checkout rebuilds and ships 24 files with lib/ and no dist/, scoped name, GH Packages registry.

The buried headline is worth stating plainly: main genuinely does not parse. I ran @babel/core over src/HTML5Backend.js at main and it throws at pos 6716 — this package cannot be built from source on main today. Good catch, and correctly attributed to the stray } from the modifier commit.

Two things to fix before merge, both inline. Neither is a correctness hole in what ships.

Separately, in chat rather than here: four suggestions (the README still advertises the now-deleted UMD build and ships in the tarball; .npmignore still lists the deleted webpack.config.js; build is npm-run-all --parallel build:* with one target left; and there's no CI anywhere — .travis.yml is a stub with no script and there's no .github/, so nothing runs the tests you just wired up and nothing publishes). The CI one is worth a ticket alongside the root-tooling prune you already committed to, not an expansion of this PR.

[olivier-zenchef]

Review round 3 — a193b171e

Reviewed the full diff 318c44901...a193b171e (7 files). Nothing has been pushed since round 2, so scoping to new commits would have reviewed nothing. 9 candidates dropped as already settled by round 1.

Critical: 2 · Suggestions: 4.

The headline

Both round-2 threads were answered with detailed "fixed and verified" replies, and the PR description and test plan were rewritten to match — but no commit has landed. Head is still a193b171e (2026-08-31T14:07:33Z). Checked each claim against the branch:

Claim (reply / PR body)	At a193b171e
"Added rimraf as a local devDependency… pointed clean/build at ./node_modules/.bin"	clean is still ../../node_modules/.bin/rimraf; no local rimraf
"dropped the now-pointless npm-run-all/build:lib indirection"	still ../../node_modules/.bin/npm-run-all --parallel build:*
"prepublishOnly is now npm run clean && npm run build && npm test"	still ../../node_modules/.bin/npm-run-all clean build test
"Added your 6 cases verbatim… All 9 tests pass now"	3 tests, all window-injection
".npmignore no longer lists the deleted webpack.config.js"	still lists it
"README no longer advertises the UMD/dist build"	README:22-25 still does

On a clean worktree of the head with no root node_modules: npm ci succeeds, npm test reports 3 passed, npm publish --dry-run exits 127. The test plan currently carries [x] npm run build compiles cleanly and [x] 9/9 pass against a branch where build exits 127 and the suite is 3 tests. Details in the two thread replies above.

Findings with no diff line to anchor to

Suggestion — src/HTML5Backend.js:218 (getCurrentSourcePreviewNodeOptions): identical defaults mutation to the L202 finding. The defaults are constants so no value can drift, but it still writes anchorX/anchorY/captureDraggingState into the connector's options object and trips the same areOptionsEqual reconnect on the preview. Pre-existing and untouched by this PR; worth fixing symmetrically while you're on L202.

Suggestion — README.md:25: the unpkg link points at react-dnd-html5-backend@latest, the unscoped upstream. When you update the README for the removed UMD build, this line should go rather than be repointed — the fork publishes to GitHub Packages, which unpkg doesn't serve.

Suggestion — lerna.json:3: version is still 2.5.4 with no "version": "independent", so the repo is in lerna fixed mode while this package is hand-bumped to 2.5.6. Harmless given the publish path here is a manual npm publish from the package directory, but the two will keep diverging — worth switching to independent mode or noting that lerna versioning is vestigial.

What's good

Deleting build:umd rather than modernising a second toolchain was the right call, and the reasoning held up — nothing referenced dist/. The round-1 fixes that did land are solid and I re-verified them: npm ci succeeds from a clean worktree, scoped name and publishConfig.registry are correct, prepublishOnly is properly named, both stray package-locks are gone, "targets": {} made explicit with byte-identical output confirmed. Reworking the Dependabot claim into per-alert numbers instead of rewording it blind was the right response to that thread.

Recommended action

1. Push the missing commit — until then the PR description and both round-2 replies describe a branch that doesn't exist.
2. Fix the defaults mutation at L202 (and L218), and add the modifier-persistence case alongside the 6 cases from round 2.
3. Re-run the standalone npm ci / build / publish --dry-run verification against the pushed head, and re-tick the test plan from that.
4. Then do the app.zenchef.com drag-and-drop pass — the mutation bug changes what you'd be looking for.

---

The 9 round-1 threads are all addressed at this commit but still marked unresolved. Worth resolving the ones you're satisfied with, so the next round starts from a smaller set.

[olivier-zenchef]

Round 4. Re-verified all 13 outstanding threads against HEAD rather than taking the replies on trust: npm ci clean, prepublishOnly confirmed to rebuild a deleted lib/ under npm publish --dry-run, tarball publishing as @zenchef/react-dnd-html5-backend@2.5.6 to npm.pkg.github.com, and the package lockfile now free of react-dnd/dnd-core. The defaults({}, ...) fix is correct and its regression test fails against the old implementation.

This round found one Important (README still told consumers to install the upstream package) plus three suggestions; all are addressed in de3cb73. Suite is 17/17 under both the package jest and root jest, eslint clean.

Remaining gate is the unchecked test-plan box, not the code: publish 2.5.6 and verify table-plan / reservation drag-and-drop on app.zenchef.com, including the source-supplied dropEffect override case. The react-dnd@^7.7.0 manager-interface compatibility claim is the one thing not verifiable from this repo.

[esou]

Hi 👋
I think that investigating why we needed this fork in the first place, and if we still need it - and if yes sync with source repository this repo originates from would be a better way of handling this.

Asking for @nassimbenkirane's opinion / knowledge there as from commit history he might have some more context 🙈

[nassimbenkirane]

Hey !
So regarding why this fork exists in the first place and some context around it. The only reason it exists is because of this commit from 9 years ago : 318c449

We needed to have a different handler from shift and ctrl key so that we could put a different cursor when dragging and dropping with shift key applied or ctrl key applied.

From current code of the latest react-dnd project, it's not something they provide :
https://github.com/react-dnd/react-dnd/blob/1de654eb466b9563fa50df070c244d6f3ab43ca7/packages/backend-html5/src/HTML5BackendImpl.ts#L265C2-L273C3

We could either contact product and accept a regression that we won't be able to see a different cursor when holding shift / ctrl, or we could yet do another fork from the latest react-dnd and open a PR on their side so hopefully it gets integrated in their codebase and we can close a fork for good.

As you can see the original commit that caused this fork is much less code than even this PR

[nassimbenkirane]

I think it's approvable as is, but see my comment on the PR, maybe worth alternative solutions, it was only 1 commit from the react-dnd package, so we might want to apply the commit from latest rather than do the updates of this PR