-
-
Notifications
You must be signed in to change notification settings - Fork 3.9k
BREAKING CHANGE: remove browser build, move to @mongoosejs/browser instead #15385
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR removes all browser‐specific builds and drivers from the core codebase and shifts browser support to the separate @mongoosejs/browser repo. It also includes adjustments such as changing the async_hooks import to remove the "node:" prefix and refactoring document middleware handling so that filtered middleware is stored directly on the document.
Reviewed Changes
Copilot reviewed 19 out of 21 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| test/files/sample.js | Removed the browser test sample to clean up obsolete browser builds. |
| test/docs/lean.test.js | Added a workaround (deleting $__middleware) to prevent v8Serialize() from crashing. |
| test/deno_mocha.js | Eliminated exclusion of browser tests (as those files are now removed). |
| lib/schema.js | Introduced a new helper (_getDocumentMiddleware) for filtering out non‑document hooks. |
| lib/mongoose.js | Changed the async_hooks import from node:async_hooks to async_hooks. |
| lib/helpers/model/applyHooks.js | Refactored hook extraction to use the new _getDocumentMiddleware function. |
| lib/document.js | Updated document initialization to set $__middleware and revised updateOne hook usage. |
| (Others: lib/drivers/browser*, lib/browser.js, lib/browserDocument.js, lib/documentProvider.js) | Removed obsolete browser-specific drivers and utilities. |
| docs/nextjs.md | Updated documentation regarding Edge Runtime to reflect removal of the browser build. |
Files not reviewed (2)
- package.json: Language not supported
- test/files/index.html: Language not supported
Comments suppressed due to low confidence (3)
lib/mongoose.js:42
- Ensure that using 'async_hooks' without the 'node:' prefix is compatible with your targeted Node.js environments, as this change is critical for the browser build removal.
const { AsyncLocalStorage } = require('async_hooks');
lib/document.js:3657
- Confirm that all documents receive the correct middleware by ensuring that $__middleware is properly initialized in $__setSchema; consider adding a unit test to verify middleware execution paths.
this.$__middleware = schema._getDocumentMiddleware();
lib/helpers/model/applyHooks.js:82
- [nitpick] Refactoring hook extraction with _getDocumentMiddleware improves maintainability; ensure that all edge cases covered in the previous manual filtering logic are preserved in this new abstraction.
const middleware = schema._getDocumentMiddleware();
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
Also worth noting there will be some followup work to this PR necessary for #15154. |
hasezoey
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me, though this should likely also be noted in the migration guide.
Some other questions though:
- Do i understand correctly that the browser-build plugin is basically now just a different driver instead of having extra stuff like a
browserDocument? - Will the new documentation be on
https://mongoosejs.com/docs/or as other things inhttps://plugins.mongoosejs.io/?
|
Fix #15296
Summary
Moved the browser build and browser driver into this separate GitHub repo: https://github.com/mongoosejs/mongoose-browser. This lets us delete a lot of code and remove a lot of dependencies.
Outside of removing the browser lib, there's a couple of other noteworthy changes:
require('node:async_hooks')torequire('async_hooks')as a slight workaround for the browser lib. Webpack complains about not being able to handlenode:imports.this.$__middlewarethat is set in$__setSchemathat we use for executing document middleware. Previously, we relied onapplyHooks()to set_middlewareon theModel, which would contain a filtered copy ofschema.s.hooksthat just contained document middleware. However, that caused trouble for the browser lib, and caused us trouble in some other places as well. The current approach of setting the filtered document middleware on the document (or document prototype) itself when$__setSchema()is called seems more robust because that ensures that a document has a$__middlewarewhenever it has a schema.Still todo:
[ ] Move
browser.mddocs to mongoose-browser and makebrowser.mdlink out.Examples