Conversation
matux
force-pushed
the
matux/cjs-to-esm
branch
2 times, most recently
from
415d087 to
4d1b153
Compare
matux
changed the title
matux
force-pushed
the
matux/cjs-to-esm
branch
2 times, most recently
from
8484bc9 to
9fed9ab
Compare
matux
changed the title
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR fully migrates the codebase from CommonJS to ES modules, updating imports/exports and related configurations.
- Converted
require/module.exportstoimport/exportin all source files - Added
"type": "module"topackage.jsonand updated build/test configs to.cjsextensions - Adjusted bundle snippets and examples to use ES module syntax
Reviewed Changes
Copilot reviewed 89 out of 89 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/browser/rollbar.js | Replaced CommonJS requires with ES import and default export |
| src/browser/predicates.js | Switched to named ES imports and exports |
| src/browser/logger.js | Imported polyfill and utilities via ES modules, default export logger |
| src/browser/globalSetup.js | Changed to named ES exports |
| src/browser/domUtility.js | Converted to named ES exports |
| src/browser/detection.js | Switched to default ES export |
| src/browser/defaults/scrubFields.js | Changed default export to ES module |
| src/browser/core.js | Updated all dependencies to ES imports, default export core |
| src/browser/bundles/rollbar.snippet.js | Switched bundle snippet to ES imports |
| src/browser/bundles/rollbar.noconflict.js | Converted to ES import and default export |
| src/browser/bundles/rollbar.js | Updated bundle entry to ES modules |
| src/apiUtility.js | Converted to named ES imports and exports |
| src/api.js | Switched to ES imports and default export |
| package.json | Added "type": "module" |
| karma.conf.cjs | Updated require paths to .cjs files |
| examples/node-dist/index.cjs | Updated source map reference to .cjs.map |
| bower.json | Updated file list to reference .cjs configs |
| Gruntfile.cjs | Updated webpack and karma config file references to .cjs |
Comments suppressed due to low confidence (2)
src/browser/defaults/scrubFields.js:1
- The default export here is an object containing a
scrubFieldsproperty, but consumers (e.g., incore.js) import it as if it were the array itself. Consider exporting the array directly (export default [ ... ]) or use a named export and adjust the import toimport { scrubFields }.
export default {
src/browser/logger.js:2
- [nitpick] Verify that
detectionis used in this file. If it's no longer needed after the migration, remove this import to avoid unused dependencies.
import detection from './detection.js';
brianr
reviewed
Jun 16, 2025
|
|
||
| /* eslint-disable no-console */ | ||
| var logger = { | ||
| const logger = { |
Contributor
Author
There was a problem hiding this comment.
We should make a whole var -> const/let migration after.
brianr
reviewed
Jun 16, 2025
brianr
approved these changes
Jun 16, 2025
Member
brianr
left a comment
brianr
left a comment
There was a problem hiding this comment.
Read each change. Looks good to me. Added one non-blocking question.
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.
Description of the change
This PR fully migrates the codebase from CommonJS to ES modules, updating imports/exports and related configurations.
Related issues
SDK-492/migrate-source-code-to-es-modules