misc: address a few esmodule TODOs #14258
Conversation
|
For some reason I can't get locales to work with a locally built devtools (only english is shown for all of devtools, not related to these changes). Should port the now-removed i18n test to e2e first before merging this. |
| @@ -1,7 +1,6 @@ | |||
| { | |||
| "extends": "./tsconfig-base.json", | |||
| "compilerOptions": { | |||
| // TODO(esmodules): included to support require('file.json'). Remove on the switch to ES Modules. | |||
| "resolveJsonModule": true, | |||
There was a problem hiding this comment.
We have a number of @type comments that import json files for a type.
build/build-report.js
Outdated
| delimiters: ['', ''], | ||
| values: { | ||
| 'getModuleDirectory(import.meta)': '""', | ||
| }, |
There was a problem hiding this comment.
inline-fs replaces the entire expression using moduleDir anyway, so this doesn't matter ... except to remove invalid syntax/code from the bundled result. Ideally, a bundler would drop const moduleDir = ... entirely (and the import of getModuleDirectory entirely, since unused) ... but rollup isn't doing that for us. Maybe esbuild would :)
Or maybe it just need some sort of pure comment? idk. for now, easier to just rip it out like this.
| @@ -52,11 +51,26 @@ async function main() { | |||
| ], | |||
| javascripts: [ | |||
| reportGeneratorJs, | |||
| 'window.ReportGenerator = window.ReportGenerator.ReportGenerator', | |||
There was a problem hiding this comment.
Can you add a TODO to remove this for #13429
connorjclark
changed the title
This PR is meant to be merged without squashing.
Going through some of the
TODO(esmodules)comments, each commit here addresses a separate thing (so you should view each commit in isolation)EDIT: converted to draft, a few unexpected test failures to dig through first