-
Notifications
You must be signed in to change notification settings - Fork 4
Move node-cli to be pure ESM, use Conf for persistent storage #203
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
Changes from 33 commits
67e1402
3a739b4
c3577ce
d3f9ea9
f29e56b
81edc40
3690ccb
4d3339a
13d7cde
1f1f120
a98f439
b3114a9
2e462ee
f9f9f6d
7dce78b
17f9e6c
da4ba81
77aba02
35dd5ff
8a7aae6
2519feb
fdefb4d
3d2c41e
0cba13f
45a1ac1
04640d5
aff3dae
5289bb3
e531e3e
e6e1c4f
f6b9dc4
aadfada
5bcda63
c33fc77
0d2c008
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| { | ||
| "type": "major", | ||
| "comment": "Move from cjs to esm and use Conf", | ||
| "packageName": "@itwin/node-cli-authorization", | ||
| "email": "50554904+hl662@users.noreply.github.com", | ||
| "dependentChangeType": "patch" | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -11,15 +11,15 @@ | |
| import { readFileSync } from "fs"; | ||
| import * as path from "path"; | ||
| import * as Http from "http"; | ||
| import * as open from "open"; | ||
| import open from "open"; | ||
| import { assert, BeEvent, BentleyError, Logger } from "@itwin/core-bentley"; | ||
| import { | ||
| AuthorizationError, AuthorizationNotifier, AuthorizationRequest, AuthorizationRequestHandler, AuthorizationResponse, | ||
| AuthorizationServiceConfiguration, BaseTokenRequestHandler, BasicQueryStringUtils, GRANT_TYPE_AUTHORIZATION_CODE, GRANT_TYPE_REFRESH_TOKEN, | ||
| TokenRequest, | ||
| } from "@openid/appauth"; | ||
| import { NodeCrypto, NodeRequestor } from "@openid/appauth/built/node_support"; | ||
| import { TokenStore } from "./TokenStore"; | ||
| import { NodeCrypto, NodeRequestor } from "@openid/appauth/built/node_support/index.js"; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. do we really need to modify imports of our deps to point to the js file?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. unfortunately, yes... Specifying See the comments on this post on an explanation on what goes on underneath the compiler, and see this github discussion on the TS devs being tired of explaining their reasoning 😢
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Seems like you can replicate the effects from this flag --experimental-specifier-resolution=node by creating a custom loader for node but yeah that sounds much more troublesome than just adding .js extension 😁
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Reading all the comments about this, it's starting to make sense... including the explicit file extension improves module resolution performance (the loader doesn't need to go through every permutation of |
||
| import { TokenStore } from "./TokenStore.js"; | ||
|
|
||
| import type { AccessToken } from "@itwin/core-bentley"; | ||
| import type { AuthorizationClient } from "@itwin/core-common"; | ||
|
|
@@ -152,8 +152,6 @@ export class NodeCliAuthorizationClient implements AuthorizationClient { | |
| // Would ideally set up in constructor, but async... | ||
| if (!this._configuration) | ||
| this._configuration = await AuthorizationServiceConfiguration.fetchFromIssuer(this._bakedConfig.issuerUrl, new NodeRequestor()); | ||
|
|
||
| await this._tokenStore.initialize(); | ||
| } | ||
|
|
||
| private async loadAndRefreshAccessToken() { | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.