Repository navigation
fix(ws,microservices): load packages where bundlers can see them - #18285
Merged
Merged
Conversation
ws is a dependency of @nestjs/platform-ws, but the adapter loaded it
with createRequire(import.meta.url)('ws') in its constructor, which
esbuild and rolldown don't follow: a bundled app left ws out and exited
with "ws package is missing". A static import is bundled by all of them.
ClientMqtt, ClientRMQ, ClientNats, ServerGrpc and ServerRMQ loaded their packages with createRequire() in the constructor, which esbuild and rolldown don't follow. They now load them with a literal import() when the client connects or the server listens, as ServerKafka and ServerRedis already do. createClient() of ClientMqtt and ClientRMQ still loads the package when called before connect(). ClientGrpcProxy keeps its synchronous load, since getService() is synchronous; bundled applications have to mark its packages external.
webpack 5 follows createRequire(import.meta.url)('pkg'), esbuild and
rolldown don't. Point to a literal import() loader or a static import.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
WsAdapterand several microservices transports load their packages withcreateRequire(import.meta.url)('pkg')in their constructors. webpack 5 follows that, but esbuild and rolldown don't (evanw/esbuild#1828): the package is left out of the bundle without a warning, and a bundled app that runs withoutnode_modulesexits withThe "ws" package is missing. ClientMqtt, ClientNats, ClientRMQ and ServerGrpc fail the same way, even when the package is in the bundle through another import. Under--format=cjs,import.meta.urlis empty, so these loads fail even when the package is installed. This came with the ESM migration in v12; v11 used() => require('pkg'), which esbuild could see.Issue Number: Closes #18284
Thanks to @sumanthratna for the report and the esbuild reproduction.
What is the new behavior?
@nestjs/platform-wsimportsWebSocketServerfromwsstatically.wsis a regular dependency of the package, so it's always bundled.connect(), and ServerGrpc and ServerRMQ when they listen, with a literalimport(), as ServerKafka and ServerRedis already do. Aclose()during the first load cancels the connection attempt instead of leaving a client open.createClient()of ClientMqtt and ClientRMQ still loads the package if it's called beforeconnect().KafkaReplyPartitionAssignerreuses the kafkajs that ClientKafka has already loaded.loadPackageSyncdocstring now says which bundlers followcreateRequireloaders.Checked with esbuild 0.28.2 (ESM output, each bundle run from a folder without
node_modules): before,new WsAdapter(server)exited with "ws package is missing" and ServerGrpc bundled 0 grpc-js modules; after, ws is in the bundle, ServerGrpc bundles grpc-js and binds, and ClientMqtt and ClientNats connect to local brokers.Does this PR introduce a breaking change?
A missing transport package is now reported (same message, same exit) when the client connects or the server starts listening, instead of when it is constructed.
Other information
@grpc/grpc-jssynchronously, becausegetService()is synchronous. Like theprotoLoaderoption (whose package name is only known at runtime), it can't be bundled by esbuild or rolldown: mark these packages as external (e.g.--packages=external) and keep them innode_modules.@nestjs/microservicesnow also needs@grpc/grpc-jsto be installed or marked external, as already applies to kafkajs, ioredis, mqtt, @nats-io/transport-node and amqp-connection-manager.wsis CommonJS, so bundling it into ESM output with esbuild needs arequirebanner, e.g.--banner:js="import { createRequire } from 'node:module'; const require = createRequire(import.meta.url);".