Import jsons directly instead of reading file

[doteric]

So I'm curious why are you using directory paths and reading files instead of directly importing JSONs using import?
https://github.com/GoogleChrome/lighthouse/blob/main/shared/localization/locales.js#L31
I'm not sure on why is this approach taken, but I guess there is a reason?

So for example an updated version could look like this:

import arLocale from './locales/ar.json';
...

And let the bundler (rollup here? in my case esbuild) handle this correctly.

The current method causes esbuild to not package the JSONs together with the bundle and results in errors.

ENOENT: no such file or directory, open '/var/task/locales/ar.json'

And also any suggested workaround for this?

[doteric]

So my preferred workaround approach was to split my code from the dependencies by create an AWS Lambda Layer. I've automated this by creating a lambda_layer/package.json like so:

{
  "dependencies": {
    "@sparticuz/chromium": "^112.0.2",
    "lighthouse": "^10.1.0"
  }
}

And ten via CI:

  script:
    - cd lambda_layer
    - yarn install --production=true
  artifacts:
    paths:
      - lambda_layer/node_modules/
    expire_in: 60 mins

And the rest is terraform code to create the zip, upload it to s3 and use it for the new lambda layer 😁
Just writing it here if somebody stumbles upon the same problem in the fututre.

But still... It might be worth changing the import approach in this project.

[adamraine]

I don't remember the exact design decision. Right now we are using a custom fs inline plugin to handle this for our bundles. Maybe @brendankenny or @connorjclark can comment if this was supposed to be a temporary measure.

In any case, we don't really support bundling out of the box. I think the most "supported" method would be to create a lighthouse bundle with our bundle script and provide your entry point as the first parameter.

[doteric]

Hey @adamraine, thanks for the reply 💪

esbuild default and recommended behavior is to always bundle, but anyway my solution to just split it into a separate layer works totally fine so all good, just curious on the import approach and suggesting that it might be worth to convert to esm style imports 😁

[connorjclark]

This is not valid JavaScript / ES Modules:

import arLocale from './locales/ar.json';

Bundlers that support the above are doing their own thing. It has to be this:

import arLocale from './locales/ar.json' assert { type: `json` };

Which Node only supports by default in 17.5.0. Lighthouse is currently on 16.16

[brendankenny]

FWIW, support was backported to Node 16.15, but in terms of the question of why the code does this, yeah, when it was being written json imports were in flux and not supported in all the versions of Node where they would be needed.

Note that it's also still in flux due to cross-platform issues. import arLocale from './locales/ar.json' assert {type: 'json'}; will still be supported (for now?), but long term it will be import arLocale from './locales/ar.json' with {type: 'json'}; https://github.com/tc39/proposal-import-attributes

See #14611 if you want to track possible future switching to json imports where it makes sense.