refactor: replace `once` package with internal implementation
This PR removes the external once package and replaces it with a modern, internal utility function. The goal is to reduce unnecessary dependencies and simplify the codebase without altering runtime behavior.
- Removes two dependencies from the dependency tree:
onceandwrappy - Updates server startup logic (e.g.,
app.listen) to use the new localoncefunction
No functional changes expected — behavior remains consistent with the previous implementation.
🔎 Originally introduced in expressjs/express#3216
@bjohansebas I'm sorry I missed the earlier PR. I didn't get a chance to review it before it was closed. I thought we had a policy about leaving PRs open long enough to allow contributors to comment - maybe we can revisit that if needed.
I'd like to respond to a few points raised in #6555:
Removing this dep doesn't achieve anything because it is also used in other libraries that ship with express. We would need to duplicate this code into all of them.
I double-checked all 3 orgs, and as far as I can tell, express is the only package in our ecosystem depending on once. I also ran npm ls once across a few bigger company projects, and express is the only dependency bringing it in.
- https://github.com/search?q=org%3Aexpressjs+once&type=code
- https://github.com/search?q=org%3Apillarjs+once&type=code
- https://github.com/search?q=org%3Ajshttp+once&type=code
Thanks for the PR! The change is valid, and the implementation looks fine. That said, I'd stick with the current use of once. It was added intentionally in https://github.com/expressjs/express/pull/3216 as a minimal, well-scoped alternative to ee-first. I prefer to keep small, purpose-built dependencies like this when they clearly express intent and don’t add meaningful overhead. Inlining this logic doesn’t materially improve clarity or reduce maintenance cost, so I'd opt to keep it as is.
Totally understand that reasoning. However, once is only used once in the entire repo, and pulling in two external dependencies (once and wrappy) for such a small utility seems unnecessary - especially when the equivalent logic can be clearly expressed inline in modern JavaScript. If this logic were shared across multiple packages or reused in multiple places, I’d agree there’s more justification. But in this case, I believe simplifying it locally is a reasonable tradeoff.
Please don’t close this PR just yet.
I'd really appreciate it if we could keep the discussion open a bit longer to gather more feedback. Let’s keep the conversation respectful and focused on what’s best for the project long-term.
Thanks!
my position on this is unchanged. please see these particular comments:
- https://github.com/expressjs/express/pull/6555#issuecomment-2937718853
- https://github.com/expressjs/express/pull/6555#issuecomment-2940459476
pulling in two external dependencies (once and wrappy) for such a small utility seems unnecessary
pulling in dependencies is not a bad thing, and eliminating dependencies is not necessarily a good thing
note also that the once library has a test suite, and this PR does not add any unit test coverage. (tests are generally required for PRs). that's not me saying to add tests now -- because I don't think we should land this change anyway. I'm only pointing out that tests are missing
in any case, as I said in the comment linked above, I think that further discussion is better suited for an actual discussion
I still believe we shouldn't remove this dependency. I know my position isn't reflected in the PR, but I did share it on Slack, where I agreed with Jordan and, in general, with the other members who are opposed to removing this dependency to add it directly here.