Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: use correct export syntax in typings #127

Closed
wants to merge 1 commit into from

Conversation

G-Rath
Copy link
Contributor

@G-Rath G-Rath commented May 10, 2020

This should be merged before #126.

Also to export things like the interfaces, they'll need to be wrapped in a namespace:

declare namespace Chromium {
   ...
}

In TS module.exports syntax is represented by export =.

The current typings will work fine if you use the default import syntax, but will fail at runtime if you use star/namespace syntax.

By using the proper export syntax, doing import * as chromium from 'chrome-aws-lambda'; will result in an error:

TS2497: This module can only be referenced with ECMAScript imports/exports by turning on the 'esModuleInterop' flag and referencing its default export.

@G-Rath
Copy link
Contributor Author

G-Rath commented May 16, 2020

@alixaxel any chance of getting a review on this?

Copy link
Owner

@alixaxel alixaxel left a comment

Choose a reason for hiding this comment

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

@G-Rath For me, the export = Chromium; is always yielding errors:

image

For the first (chromiumDefault) case:

index.d.ts(20, 1): This module is declared with using 'export =', and can only be used with a default import when using the 'esModuleInterop' flag.

And for the second one (chromiumStar);

This module can only be referenced with ECMAScript imports/exports by turning on the 'esModuleInterop' flag and referencing its default export.ts(2497)

Do you experience the same?

@@ -1,17 +1,20 @@
import { Browser, BrowserFetcher, ChromeArgOptions, ConnectOptions, FetcherOptions, LaunchOptions } from 'puppeteer';
Copy link
Owner

Choose a reason for hiding this comment

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

@G-Rath I guess this is no longer needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, weird wonder why my IDE didn't flag it 🤔

};
static get executablePath(): Promise<string>
static get headless(): boolean;
static get puppeteer(): typeof import('puppeteer');
Copy link
Owner

Choose a reason for hiding this comment

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

@G-Rath Nicely done. 👍

@G-Rath
Copy link
Contributor Author

G-Rath commented May 19, 2020

@alixaxel yes, because it's a commonjs export, so you need esModuleInterop or to use the require import syntax:

import chromeAwsLambda = require('chrome-aws-lambda');

You can see this comment for some more info on what the import/export syntaxes compile into/from :)

alixaxel added a commit that referenced this pull request Aug 2, 2020
@alixaxel alixaxel closed this Aug 2, 2020
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