Skip to content

Conversation

@juliawu
Copy link
Contributor

@juliawu juliawu commented Dec 11, 2025

As part of #5835 , the target in tsconfig.json needs to be upgraded from es6 to es2021. This PR isolates the es2021 change.

The main difference appears to be how dynamic imports are treated. This PR:

  • Updates the module in tsconfig.json to "esnext" instead of "commonJS". This allows webpack to correctly use import() as we have across the codebase
  • Updates the handling of dynamic imports (primarily in loadLocaleData()) to be es2021 compatible
  • Includes some trailing lint fixes to get pre-merge tests to pass

@gemini-code-assist
Copy link
Contributor

Summary of Changes

Hello @juliawu, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request addresses necessary adjustments to the codebase following an upgrade of the TypeScript compilation target to es2021. The core changes involve modifying how internationalization JSON files are loaded across various parts of the application, ensuring that locale data continues to be correctly processed and applied after the tsconfig update.

Highlights

  • TypeScript Configuration Update: The TypeScript compilation target in tsconfig.json has been upgraded from es6 to es2021, enabling the use of newer ECMAScript features during compilation.
  • Internationalization (i18n) Import Adjustments: Dynamic import() statements for i18n JSON locale files (e.g., units.json, place.json) have been replaced with require() calls across numerous application entry points and utility files to ensure compatibility with the updated TypeScript target.
  • Locale Data Loading Logic Enhancement: The loadLocaleData function in static/js/i18n/i18n.tsx was updated to handle module imports more robustly by checking msg.default || msg, accommodating potential differences in how import() and require() expose JSON modules.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

@juliawu juliawu changed the title Update i18n json imports after upgrading tsconfig target to es2021 [DO NOT MERGE]Update i18n json imports after upgrading tsconfig target to es2021 Dec 11, 2025
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request updates the way i18n JSON files are imported, switching from dynamic import() to require(). This change is applied consistently across numerous files and is a consequence of upgrading the TypeScript target to es2021 in tsconfig.json. The related adjustment in i18n.tsx to handle both module formats is also correct.

My main feedback is a suggestion to improve type safety. The switch to synchronous require() calls causes a mismatch with the loadLocaleData function's type signature, which expects an array of Promises. I've left a specific comment with a recommendation to update the function signature to align with its new usage.

.then((messages) => {
for (const msg of messages) {
Object.assign(allMessages, msg.default);
Object.assign(allMessages, msg.default || msg);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

While this change correctly handles modules loaded via require, the calls to loadLocaleData throughout the codebase now pass synchronous require() results, which are not Promises. This violates the function's type signature for the modules parameter (Promise<Record<any, any>>[]).

To maintain type safety, please update the signature of loadLocaleData to reflect its new usage. For example:

function loadLocaleData(
  locale: string,
  modules: (Promise<Record<any, any>> | Record<any, any>)[]
): Promise<void> {

This will ensure the function signature accurately represents that it can handle both promises (from dynamic import()) and direct objects (from require()).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please fix

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed!

function loadLocaleData(
locale: string,
modules: Promise<Record<any, any>>[]
modules: Promise<Record<string, any>>[]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just to double check, we don't pass other types other than string to modules in Record's first arg right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct, as far as I'm aware, we only use string.

import axios from "axios";
import _ from "lodash";
import { URLSearchParams } from "url";
// import { URLSearchParams } from "url";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If not in use, can we remove this line?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes! Thanks for the catch. I was trying to do some edits via JetSki/Antigravity and it looks like this particular change didn't sync. Removed.

Copy link
Contributor

@shixiao-coder shixiao-coder left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM in general, just a few comments

@juliawu juliawu changed the title [DO NOT MERGE]Update i18n json imports after upgrading tsconfig target to es2021 Update tsconfig.json target to "es2021" and module to "esnext" Dec 12, 2025
@juliawu juliawu marked this pull request as ready for review December 12, 2025 20:10
@juliawu juliawu merged commit e76432a into datacommonsorg:master Dec 12, 2025
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants