-
Notifications
You must be signed in to change notification settings - Fork 207
Use a global cache for the sessions to avoid multiple instances #6469
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
base: main
Are you sure you want to change the base?
Conversation
9ba7fb1
to
9414441
Compare
9414441
to
a1aa2c9
Compare
Differences in type declarationsWe detected differences in the type declarations generated by Typescript for this branch compared to the baseline ('main' branch). Please, review them to ensure they are backward-compatible. Here are some important things to keep in mind:
New type declarationsWe found no new type declarations in this PR Existing type declarationspackages/cli-kit/dist/public/node/session.d.ts@@ -6,6 +6,41 @@ export interface AdminSession {
token: string;
storeFqdn: string;
}
+/**
+ * Session Object for developer platform clients, includes token and account info.
+ */
+export interface Session {
+ token: string;
+ businessPlatformToken: string;
+ accountInfo: AccountInfo;
+ userId: string;
+}
+export type AccountInfo = UserAccountInfo | ServiceAccountInfo | UnknownAccountInfo;
+interface UserAccountInfo {
+ type: 'UserAccount';
+ email: string;
+}
+interface ServiceAccountInfo {
+ type: 'ServiceAccount';
+ orgName: string;
+}
+interface UnknownAccountInfo {
+ type: 'UnknownAccount';
+}
+/**
+ * Type guard to check if an AccountInfo is a UserAccountInfo.
+ *
+ * @param account - The account info to check.
+ * @returns True if the account is a UserAccountInfo, false otherwise.
+ */
+export declare function isUserAccount(account: AccountInfo): account is UserAccountInfo;
+/**
+ * Type guard to check if an AccountInfo is a ServiceAccountInfo.
+ *
+ * @param account - The account info to check.
+ * @returns True if the account is a ServiceAccountInfo, false otherwise.
+ */
+export declare function isServiceAccount(account: AccountInfo): account is ServiceAccountInfo;
/**
* Ensure that we have a valid session with no particular scopes.
*
@@ -86,4 +121,5 @@ export declare function ensureAuthenticatedBusinessPlatform(scopes?: BusinessPla
*
* @returns A promise that resolves when the logout is complete.
*/
-export declare function logout(): Promise<void>;
\ No newline at end of file
+export declare function logout(): Promise<void>;
+export {};
\ No newline at end of file
|
Coverage report
Show new covered files 🐣
Show files with reduced coverage 🔻
Test suite run success3331 tests passing in 1382 suites. Report generated by 🧪jest coverage report action from a1aa2c9 |
We detected some changes at Caution DO NOT create changesets for features which you do not wish to be included in the public changelog of the next CLI release. |
export function isServiceAccount(account: AccountInfo): account is ServiceAccountInfo { | ||
return account.type === 'ServiceAccount' | ||
} | ||
export {AccountInfo, isUserAccount, isServiceAccount} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need to re-export them here no? Whoever is using these can directly import from cli-kit
WHY are these changes introduced?
We are caching the current session for each Developer Platform client (Partners and App Management) as an instance variable, but we are creating instances from multiple places.
That could lead in multiple login requests during the same command. I think it's not happening right now, but it could happen eventually after some refactor.
WHAT is this pull request doing?
PartnersSession
toSession
because it was shared by both clients, and moves it to cli-kitHow to test your changes?
Log in and run commands
Measuring impact
How do we know this change was effective? Please choose one:
Checklist