chore: add ESM type: module to package.json#109319
Conversation
tsconfig.json
Outdated
| "include": [ | ||
| "*.config.mjs", | ||
| "*.config.js", | ||
| "*.config.ts", |
There was a problem hiding this comment.
[Question] Could we just go with *.config.* or *.config.*s? Was the having two includes with explicit extensions intentional?
There was a problem hiding this comment.
🤷 *.config.* seems fine, just less explicit
105ecca to
80045d4
Compare
80045d4 to
be0456c
Compare
be0456c to
8dabcdf
Compare
8dabcdf to
a93a9c6
Compare
natemoo-re
left a comment
There was a problem hiding this comment.
I'm impressed with how little this seems to break so far! Looking good.
tsconfig.json
Outdated
| "include": [ | ||
| "*.config.mjs", | ||
| "*.config.js", | ||
| "*.config.ts", |
There was a problem hiding this comment.
🤷 *.config.* seems fine, just less explicit
a93a9c6 to
6c3cfcf
Compare
|
|
6c3cfcf to
f47bd88
Compare
|
🚨 Warning: This pull request contains Frontend and Backend changes! It's discouraged to make changes to Sentry's Frontend and Backend in a single pull request. The Frontend and Backend are not atomically deployed. If the changes are interdependent of each other, they must be separated into two pull requests and be made forward or backwards compatible, such that the Backend or Frontend can be safely deployed independently. Have questions? Please ask in the |
f47bd88 to
b190f10
Compare
natemoo-re
left a comment
There was a problem hiding this comment.
We should have done this much sooner! Nice work 🎉
793b2f7 to
0883b84
Compare
| @@ -1,4 +1,4 @@ | |||
| 'use strict'; | |||
| import {run} from 'jest'; | |||
There was a problem hiding this comment.
ESM import hoisting defeats env var initialization order
Low Severity
The static import {run} from 'jest' on line 1 is hoisted and evaluated before the process.env assignments on lines 5–7. In the original CJS version, require('jest') was called at the end of the file (after all env setup), matching the comment "Do this as the first thing so that any code reading it knows the right env." Now the jest module (and its transitive dependencies) loads before NODE_ENV, TZ, etc. are set, and the unhandledRejection handler is also registered after the import. A dynamic await import('jest') or moving the env setup to a separate preloaded module would preserve the original execution order.
There was a problem hiding this comment.
I don't think this matters? Jest always runs in test mode, and PUBLIC_URL & TZ are Sentry - only constructs that I know of.
Replace hand-rolled findJsonFiles with Node's built-in fs.promises.glob, and convert CJS require/module.exports to ESM import/export to align with the repo-wide ESM migration in #109319. Co-Authored-By: Claude <noreply@anthropic.com>
0883b84 to
e35bc84
Compare
e35bc84 to
76f982d
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
76f982d to
b24be8f
Compare
b24be8f to
bb573d2
Compare
Directly changes files from `.mjs` to ~`.mts`. I would have preferred the cleaner `.ts` but that will require adding `"type": "module"` to `package.json`, which is more work (which would be a great followup!).~ `.ts`, building on #109319's switching the package to be ESM by default. Imports are now TypeScript-style extension-less ones. Now that the plugin's files are typed, this applies a few types cleanups: * Uses [typescript-eslint's `RuleCreator`](https://typescript-eslint.io/developers/custom-rules#rulecreator) to create rules, which gives nicer typings than the built-in ESLint `Rule` * Updates node types from [`estree`](https://github.com/estree/estree) (only vanilla JS types) to [`@typescript-eslint/estree`](https://typescript-eslint.io/packages/typescript-estree) (full TS node types) * Converts JSDoc types to full TypeScript syntax This PR should have no changes to the lint rules' behavior. The only runtime code changes are for deleting unused parameters.


Switches the default module system for internal config/script files in this repo from CJS to ESM by specifying
"type": "module"in thepackage.jsonfile. In order to support that:.mjsfiles are renamed to.js, since.jsis now ESM instead of CJS.jsfiles usingmodule.exports/require(CJS) are now usingexport/import(ESM).mjs, as that extension is no longer necessaryQuick Node.js CJS/ESM primer
If you're new to the hullabaloo that is Node.js module loading: welcome! There are effectively two main ways that Node.js files are able to export & import things:
module.exports.key = value;const { key } = require("./file");export const key = value;import { key } from "./file.js";Any one file is able to use only one syntax. For legacy reasons Node.js defaults to interpreting
.jsfiles as CJS. Node.js can be told to change that (switch to ESM) in two ways:package.json, setting"type": "moduleswitches the default to ESM.cjsor.mjsextension for a file switches the file to CJS or ESM, respectivelyESM is generally/mostly considered to be a better syntax for a bunch of technical points. Because it's more explicit & less dynamic, it works better for a lot of tools.
Fixes https://linear.app/getsentry/issue/ENG-6928/add-esm-type-module-to-packagejson