-
Notifications
You must be signed in to change notification settings - Fork 211
Eliminate runtime package.json imports and reorganize defaults #1246
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 replaces runtime package.json
imports with proper ES module defaults to fix compatibility issues with modern build tools like Angular SSR. The change eliminates webpack DefinePlugin dependencies and moves all default configuration values to dedicated ES modules.
- Removes runtime
package.json
imports from server and React Native platforms - Creates dedicated defaults modules for shared, server, React Native, and browser configurations
- Eliminates webpack DefinePlugin usage and related build-time variable injection
Reviewed Changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 4 comments.
Show a summary per file
File | Description |
---|---|
webpack.config.cjs | Removes DefinePlugin and defaultsPlugin references from all webpack configurations |
src/server/rollbar.js | Replaces package.json imports with ES module imports from defaults files |
src/server/defaults.js | New file defining server-specific default configurations |
src/react-native/rollbar.js | Replaces package.json imports with ES module imports from defaults files |
src/react-native/defaults.js | New file defining React Native-specific default configurations |
src/defaults.js | Converts to ES modules with named exports for shared default values |
src/browser/defaults.js | New file consolidating browser-specific defaults including scrub fields |
src/browser/core.js | Updates imports to use new ES module defaults structure |
src/browser/bundles/rollbar.snippet.js | Replaces webpack-injected variable with ES module import |
package.json | Removes defaults configuration section now handled by ES modules |
defaults.cjs | Removes entire file as webpack DefinePlugin is no longer used |
79d91e3
to
792ec97
Compare
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.
I'm not sure if this is right. Is this file meant to be standalone? Would the import work in this case? Would webpack do the smart thing and bundle the actual constant instead of leaving the import? Actually haven't checked this...
Should tests fail if this didn't work?
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.
Webpack writes a file to /dist/plugins/. You can npm run build
and see if it generates the expected output.
Here is the usage doc. https://docs.rollbar.com/docs/rollbarjs-plugins
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.
Yep, it does the right thing!
t.configure({payload:{notifier:{plugins:{jquery:{version:"0.0.8"}}}}});
Description of the change
This PR eliminates runtime
package.json
imports from the server and react native platforms, replacing them with proper ES module defaults. This fixes compatibility issues with modern build tools like Angular SSR, which cannot resolvepackage.json
imports in certain contexts.The Angular example now builds successfully without any changes to the example project itself.
The
DefinePlugin
webpack dependency has been removed together with all traces of the old way of loading defaults.Type of change
Related issues
SDK-518/make-sure-all-sdk-examples-work-properly