Skip to content

Remove __non_webpack_require__ workaround and split Node dependencies correctly #48154

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jul 12, 2023

Conversation

BrennanConroy
Copy link
Member

Fixes #47674

Removed __non_webpack_require__ usage and replaced it with the browser field in package.json. And moved the dynamic require code into a swappable file. This allows us to tell bundlers not to include the external dependencies as well as provide noop/error methods in case we ever accidentally call the dynamic requires in the browser.

Tested with esbuild and an angular app.
Verified angular app doesn't include external dependencies that we use for Node in the browser
Verified esbuild calls require_* and includes external dependencies in bundle.
Verified signalr.min.js stays at 43kb.

@BrennanConroy BrennanConroy added area-signalr Includes: SignalR clients and servers feature-client-javascript Related to the SignalR JS client labels May 9, 2023
@BrennanConroy BrennanConroy requested a review from halter73 as a code owner May 9, 2023 21:35
@ghost
Copy link

ghost commented May 17, 2023

Looks like this PR hasn't been active for some time and the codebase could have been changed in the meantime.
To make sure no breaking changes are introduced, please leave an /azp run comment here to rerun the CI pipeline and confirm success before merging the change.

@ghost ghost added the pending-ci-rerun When assigned to a PR indicates that the CI checks should be rerun label May 17, 2023
@BrennanConroy BrennanConroy requested a review from mitchdenny July 11, 2023 20:04
@mitchdenny mitchdenny merged commit 93520b6 into main Jul 12, 2023
@mitchdenny mitchdenny deleted the brecon/improveRequire branch July 12, 2023 08:14
@ghost ghost added this to the 8.0-preview7 milestone Jul 12, 2023
BrennanConroy added a commit that referenced this pull request Apr 19, 2024
wtgodbe pushed a commit that referenced this pull request May 13, 2024
BrennanConroy added a commit that referenced this pull request Jul 12, 2024
BrennanConroy added a commit that referenced this pull request Jul 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-signalr Includes: SignalR clients and servers feature-client-javascript Related to the SignalR JS client pending-ci-rerun When assigned to a PR indicates that the CI checks should be rerun
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SignalR npm package not able to be bundled using esbuild
2 participants