Skip to content

FIx / improve running build-locale-data scripts locally - #406

Closed
boarnoah wants to merge 2 commits into
mainfrom
aw/run-build-locale-scripts-locally-via-npm
Closed

boarnoah wants to merge 2 commits into
mainfrom
aw/run-build-locale-scripts-locally-via-npm

Conversation

@boarnoah

@boarnoah boarnoah commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Adding support for new locales involve running scripts node /build-locale-data/build-files.js

But that fails because it is missing cldr dependency + node not having the document DOM API.

I am not sure how @bearfriend typically runs this script so I might have misunderstood. The script is running file locally now, but it generates a lot of locale-data changes?

PS: I am associating this with the vulcan card to add Gaelic support, but this might be more appropriate under a Gaudi card re: improvements in this area?

https://desire2learn.atlassian.net/browse/VUL-1398

@boarnoah boarnoah self-assigned this Aug 24, 2026
@boarnoah
boarnoah requested a review from a team as a code owner August 24, 2026 21:02
Comment thread lib/common.js
return langTag;
}

export function merge(obj1, obj2, keepOriginal = false) {

@boarnoah boarnoah Aug 24, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

this is used in build-locale-data scripts, but failing like so:

> node ./build-locale-data/build-files.js

file:///C:/D2L/Code/intl/lib/document-locale-settings.js:1
const host = document.documentElement;
             ^

ReferenceError: document is not defined
    at file:///C:/D2L/Code/intl/lib/document-locale-settings.js:1:14
    at ModuleJob.run (node:internal/modules/esm/module_job:439:25)
    at async node:internal/modules/esm/loader:643:26
    at async asyncRunEntryPointWithESMLoader (node:internal/modules/run_main:101:5)

so refactored it out, since they are more standalone util functions not dependent on DOM APIs

Comment thread package.json
"test:unit": "d2l-test-runner",
"test": "npm run lint && npm run test:unit"
"test": "npm run lint && npm run test:unit",
"build-locale-data": "node ./build-locale-data/build-files.js"

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

assuming there is a reason this wasn't already done this way? In which case, is a separate package.json for the build-locale-data more appropriate?

Comment thread package.json
"homepage": "https://github.com/BrightspaceUI/intl",
"devDependencies": {
"@brightspace-ui/testing": "^1",
"cldr": "^8.0.0",

@boarnoah boarnoah Aug 24, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

assumed latest is fine*? I do think this kind of stuff needs to be version controlled so anyone could run these locale build scripts?

* It might be the cause for these changes I see... was I supposed to use a different version?

@dlockhart
dlockhart marked this pull request as draft August 25, 2026 16:49
@dlockhart

Copy link
Copy Markdown
Member

Thanks @boarnoah -- I've converted this into a draft for now so we can wait for @bearfriend to have a look. I'm not sure if everything is in a complete state or if there was more to do.

@dlockhart

Copy link
Copy Markdown
Member

I'm going to close this for now to get it off our "to review" list, but something Danny can review when he's back.

@dlockhart dlockhart closed this Sep 18, 2026
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