Open mattpen opened 4 years ago
That looks like it would work from my perspective.
@jbphet @Denz1994 - If this went through, you could replace the module parsing in rosetta/js/localeInfo.js with a simple JSON.parse
statement.
The installer-builder-config.json field pathToLocaleInfo
would need to be updated on Bayes, but otherwise, this seems fine. I'll remain assigned so I can update the config, when ready.
It would be fine with me to do this, and I agree that it would simplify Rosetta's usage of this file. Both files - the .js
and the .json
files should exist for a while to allow me to modify Rosetta, test it, and then deploy. Once that's done, the old .js
one can be deleted (as far as I'm concerned).
I'll proceed with this change. First I'll add the json file, then when subsequent chagnes are complete I'll push the chagnes to the js file.
@Denz1994 @jonathanolson @jbphet - /data/localeInfo.json
is now available in master. Please make any adaptations if needed, remove your assignment, and if you are the last one reassign me to make the final change (changing localeInfo.js to a wrapper of the json).
Bumping priority down, as now that the json exists I can proceed with website-meteor work.
The work on Rosetta is complete and the js
version of the file can be deleted from chipper (as far as Rosetta is concerned - translation might be another matter).
the js version of the file can be deleted from chipper (as far as Rosetta is concerned - translation might be another matter)
I think the js version is still needed, but I'll change it to be a wrapper for the json file once everyone has confirmed.
I see installer-builder and yotta are still using the JS file. JSON conversion looks good.
The installer builder has been changed to use JSON.
After reviewing usage in yotta's aggregate.html
, converting the js file to use the json as a source isn't trivial. I assumed that we could use the node fs
api to retrieve the file, but yotta needs to fetch this data from a browser. We can either
(a) use the github raw api to fetch the file via http request in localeInfo.js (using a library that is compatible with node AND browsers) or
(b) modify aggregate.html
or aggregate.js
, along with yotta-server.js
to use the json file instead.
IMO (b) seems like the cleanest way to go, but I'd like @jonathanolson's input since that involves yotta changes.
Bumping priority down as this list isn't likely to change soon, so we can keep using the current solution (maintain 2 nearly identical files) for now.
Option (b) seems fine to me.
I have a need to use https://github.com/phetsims/chipper/blob/master/js/data/localeInfo.js in the website-meteor project, to show a list of locales to users in "Localized Display Name" format.
I checked the way that rosetta is using this file and it seems overly complicated. We are downloading the file via a request to the Github Raw API, then doing some processing on the resulting string to turn it into a requirejs module and importing it. Copying this logic into website-meteor seems inappropriate, and would probably not perform well in that context.
The file seems to be a wrapper of what could be a basic JSON object. Would it be ok to factor out the locale info in to a json object (stored in chipper/data/localeInfo.json), then use the nodejs fs api to make it available as an ES6 module in /js/data/localeInfo.js?
I'm assigning @Denz1994 @jbphet and @jonathanolson as it looks like rosetta and yotta have dependencies on this file.