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

ConnectorRunner should not invoke require in loadConnector #40

Open
jackson-at-bentley opened this issue Jun 20, 2022 · 0 comments
Open

Comments

@jackson-at-bentley
Copy link
Contributor

jackson-at-bentley commented Jun 20, 2022

// ConnectorRunner.ts

private async loadConnector(connectorFile: string) {
  // eslint-disable-next-line @typescript-eslint/no-var-requires
  const connectorClass = require(connectorFile).default;
  this._connector = await connectorClass.create();
}

A connector is a subtype of BaseConnector and should be passed to the connector runner as an instance.

Invoking require seems like a bad idea in an npm package for two reasons.

  • The import is relative to the file that called require. This means users of our API must specify their connector relative to node_modules, or use an absolute path.
  • Our API does not support connectors written as ES modules. Users are required to compile to CommonJS.

If we want the connector constructor to be asynchronous, we can pass the class object to the connector runner, or make create an instance method.

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 a pull request may close this issue.

1 participant