Page MenuHomePhabricator

[keyserver] Update assets.json imports to use fs instead of import
ClosedPublic

Authored by rohan on Oct 14 2022, 8:15 AM.
Tags
None
Referenced Files
F3249054: D5371.id17620.diff
Fri, Nov 15, 12:11 PM
F3248825: D5371.diff
Fri, Nov 15, 10:51 AM
Unknown Object (File)
Tue, Nov 5, 9:45 AM
Unknown Object (File)
Tue, Nov 5, 9:45 AM
Unknown Object (File)
Tue, Nov 5, 9:45 AM
Unknown Object (File)
Tue, Nov 5, 9:45 AM
Unknown Object (File)
Tue, Nov 5, 9:45 AM
Unknown Object (File)
Tue, Nov 5, 9:43 AM
Subscribers

Details

Summary

Change await import to fs.readFileSync when loading the
assets.

Test Plan

On both landing and web, ran yarn prod, then `yarn
prod-build` followed by yarn prod in keyserver. Both pages load fine.
Also changed the Node version in .nvmrc to 18.11.0 and they run fine as
well.

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

rohan requested review of this revision.Oct 14 2022, 8:25 AM
ashoat requested changes to this revision.Oct 14 2022, 8:38 AM
ashoat added inline comments.
keyserver/src/responders/landing-handler.js
64 ↗(On Diff #17580)

Can we try to use an async method here? Sorry I didn't call this out in D5267! (Would be great if you could update that callsite too)

Note you'll probably need to use promisify. Try git grep promisify to see how that works

keyserver/src/responders/website-responders.js
86 ↗(On Diff #17580)

Here too

This revision now requires changes to proceed.Oct 14 2022, 8:38 AM

Using async method to read file and use promisfy

ashoat requested changes to this revision.Oct 14 2022, 10:33 AM
ashoat added inline comments.
keyserver/src/responders/landing-handler.js
65–66 ↗(On Diff #17587)

Can we separate the await out onto its own line (see proposed edit)? I prefer to avoid wrapping await with other calls since it can "hide" the await when reading the code

Please apply this change to the other two codesites. Also worth just inlining assetsURL now that it fits

This revision now requires changes to proceed.Oct 14 2022, 10:33 AM

Separate await statement into it's own line

This revision is now accepted and ready to land.Oct 14 2022, 2:00 PM