DanielLeens commented on PR #10973: URL: https://github.com/apache/seatunnel/pull/10973#issuecomment-5556765377
Correcting my own prior review: I called the date-fns/date-fns-tz reintroduction "High severity... undoes ~36 MB / 40% of the PR's own claimed savings." That impact claim is wrong, and I want to flag it before anyone acts on it. Checked `package-lock.json` directly: `naive-ui` (a real, direct dependency of this module - imported in 17 files under `src/`) itself requires `date-fns@^3.6.0` and `date-fns-tz@^3.1.3` - the exact same ranges previously pinned at the top level, resolving to the exact same locked versions (3.6.0 / 3.2.0). Removing the top-level declaration doesn't change what `npm install` actually puts on disk: naive-ui's own manifest requires them regardless, so there's no dependency-graph edge to cut here, and no real `node_modules` size reduction from this specific change. Pushed `2f9f7341693`, which: - Removes the redundant top-level declaration from `package.json`/`package-lock.json` as a correctness fix (this module's own code never imports them directly) - not a size fix, and the PR description now says so explicitly, with the "Estimated savings" table corrected from ~90 MB to ~53 MB (dropping the ~36 MB date-fns line item). - Also fixes the other two items from this thread: `package-lock.json`'s `resolved` URLs now point at `registry.npmjs.org` instead of `registry.npmmirror.com`, and `src/components/icons/index.ts` now carries an in-file MIT attribution for the vendored Ionicons SVG data, pointing at the existing `seatunnel-dist/release-docs/licenses/LICENSE-ionicons.txt`. - Corrects the PR description's `npm install --omit=dev` claim per Issue 3 above: postcss/autoprefixer/tailwindcss/sass are devDependencies but all required at `vite build` time, so `--omit=dev` would break the build rather than optimize it. Re-review still needed on this new head once CI finishes. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
