foo-software / lighthouse-persist

Uploads a Lighthouse report to your AWS S3 account.
12 stars 3 forks source link

Error running sample code. #5

Closed daniwaxman closed 4 years ago

daniwaxman commented 4 years ago

Using node 14.13.1 trying to run the sample code as provided I get the following error: (node:20496) UnhandledPromiseRejectionWarning: TypeError: lighthousePersist is not a function at file:///c:/XXX/XXX/lighthouse/index.js:5:43 at file:///c:/XXX/XXX/lighthouse/index.js:15:3 at ModuleJob.run (internal/modules/esm/module_job.js:146:23) at async Loader.import (internal/modules/esm/loader.js:165:24) at async Object.loadESM (internal/process/esm_loader.js:68:5) (Use node --trace-warnings ... to show where the warning was created) (node:20496) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). To terminate the node process on unhandled promise rejection, use the CLI flag --unhandled-rejections=strict (see https://nodejs.org/api/cli.html#cli_unhandled_rejections_mode). (rejection id: 1)

/internal/process/warning.js:41 (node:20496) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.
adamhenson commented 4 years ago

Hi @daniwaxman - could you provide your code in its entirety? Also, did you run npm install @foo-software/lighthouse-persist?

daniwaxman commented 4 years ago

@adamhenson 1) Yes I did run the npm install. 2) I am running the sample code as is from the readme: `import path from 'path'; import lighthousePersist from '@foo-software/lighthouse-persist';

(async () => { const { localReport, result } = await lighthousePersist({ url: 'https://www.foo.software',

// example if you have an "artifact" directory one level up
outputDirectory: path.resolve(__dirname + '/../artifacts')

});

console.log({ localReport, result }); })();`

adamhenson commented 4 years ago

What does the contents of your package.json file look like?

daniwaxman commented 4 years ago

{ "name": "lighthouse", "version": "1.0.0", "description": "", "main": "index.js", "type": "module", "scripts": { "test": "echo \"Error: no test specified\" && exit 1" }, "author": "", "license": "ISC", "dependencies": { "@foo-software/lighthouse-persist": "^1.0.3" } }

adamhenson commented 4 years ago

Thanks, I'm able to reproduce now. That README documentation was based on code that was transpiled with Babel. It seems that running this code natively on Node versions that support import syntax isn't working with the example code. I'll have an answer shortly.

adamhenson commented 4 years ago

The quick fix would be to update the below code:

- const { localReport, result } = await lighthousePersist({
+ const { localReport, result } = await lighthousePersist.default({

I'm digging into why this is so... why the Node.js native behavior is different than Babel transpiled code. Will have a better update soon and will need to update the documentation. I have a feeling it relates to how we're distributing the build.

daniwaxman commented 4 years ago

Thanks - I believe you will find a similar error using __dirname

adamhenson commented 4 years ago

@daniwaxman - see the comment in the code. If you want to use that option, you'll need to create the directory. If you're just trying to get this example working - remove that line. That option allows you to specify a directory to store reports (it's optional).

daniwaxman commented 4 years ago

@adamhenson Yes- I understand it needs to be configured - I was referring to the issue that " __dirname" not a valid variable in the transpiled code. I was able to get around this using - this doc

adamhenson commented 4 years ago

Thanks again for raising this issue @daniwaxman. I updated the documentation. At this time, we're only exporting the package in CommonJS format. So, you can use the require syntax as noted in the updated README... otherwise the workaround would be my initial response. Eventually, we'll want to support Node's new ES Module format, however there are a couple caveats. I opened a new ticket for that and detailed the caveats in #6.