Skip to content

Commit b1cbfd4

Browse files
committed
PR feedback
1 parent b5e778a commit b1cbfd4

File tree

5 files changed

+24
-158
lines changed

5 files changed

+24
-158
lines changed

packages-exp/auth-exp/src/platform_browser/iframe/gapi.iframes.d.ts

Lines changed: 2 additions & 121 deletions
Original file line numberDiff line numberDiff line change
@@ -20,10 +20,7 @@ declare namespace gapi {
2020
interface LoadConfig {}
2121
interface LoadOptions {
2222
callback?: LoadCallback;
23-
config?: LoadConfig;
2423
timeout?: number;
25-
dotimeout?: LoadCallback;
26-
sync?: boolean;
2724
ontimeout?: LoadCallback;
2825
}
2926
function load(
@@ -33,9 +30,6 @@ declare namespace gapi {
3330
}
3431

3532
declare namespace gapi.iframes {
36-
interface OptionsBag {
37-
[key: string]: unknown;
38-
}
3933
interface Message {
4034
type: string;
4135
}
@@ -45,136 +39,23 @@ declare namespace gapi.iframes {
4539
message: T
4640
) => unknown | Promise<void>;
4741
type SendCallback = () => void;
48-
type StyleHandler = (options: OptionsBag) => void;
49-
type RpcFilter = (iframe: Iframe) => boolean | Promise<boolean>;
5042
type Callback = (iframe: Iframe) => void;
51-
type ContextCallback = (iframe: Iframe, done: boolean) => void;
52-
interface Params {
53-
[key: string]: unknown;
54-
}
55-
interface StyleData {
56-
[key: string]: unknown;
57-
}
58-
59-
interface IframeOptions {
60-
iframe: Iframe;
61-
role?: string;
62-
data?: unknown;
63-
isReady: boolean;
64-
}
6543

6644
class Context {
67-
constructor(options?: OptionsBag);
68-
isDisposed(): boolean;
69-
getFrameName(): string;
70-
getWindow(): Window;
71-
getGlobalParam(key: string): unknown;
72-
setGlobalParam(key: string, value: unknown): void;
73-
openChild(options: OptionsBag): Iframe;
74-
open(options: OptionsBag, callback?: Callback): Promise<Iframe>;
75-
getParentIframe(): Iframe;
76-
closeSelf(params?: Params, callback?: ContextCallback): Promise<boolean>;
77-
restyleSelf(
78-
style?: StyleData,
79-
callback?: ContextCallback
80-
): Promise<boolean>;
81-
ready<T extends Message>(
82-
params?: Params,
83-
methods?: { [key: string]: MessageHandler<T> },
84-
callback?: SendCallback,
85-
filter?: IframesFilter
86-
): void;
87-
setCloseSelfFilter(filter: RpcFilter): void;
88-
setRestyleSelfFilter(filteR: RpcFilter): void;
89-
connectIframes(
90-
iframe1Data: IframeOptions,
91-
iframe2Data?: IframeOptions
92-
): void;
93-
addOnConnectHandler(
94-
optionsOrRole: string | OptionsBag,
95-
handler?: (iframe: Iframe, obj: object) => void,
96-
apis?: string[],
97-
filter?: IframesFilter
98-
): void;
99-
removeOnConnectHander(role: string): void;
100-
addOnOpenerHandler(
101-
handler: (iframe: Iframe) => void,
102-
apis?: string[],
103-
filter?: IframesFilter
104-
): void;
45+
open(options: Record<string, unknown>, callback?: Callback): Promise<Iframe>;
10546
}
10647

10748
class Iframe {
108-
constructor(
109-
context: Context,
110-
rpcAddr: string,
111-
frameName: string,
112-
options: OptionsBag
113-
);
114-
isDisposed(): boolean;
115-
getContext(): Context;
116-
getFrameName(): string;
117-
getId(): string;
118-
getParam(key: string): unknown;
119-
setParam(key: string, value: unknown): void;
12049
register<T extends Message>(
12150
message: string,
12251
handler: MessageHandler<T>,
12352
filter?: IframesFilter
12453
): void;
125-
unregister<T extends Message>(
126-
message: string,
127-
hander: MessageHandler<T>
128-
): void;
129-
send(
130-
message: string,
131-
data?: unknown,
132-
callback?: SendCallback,
133-
filter?: IframesFilter
134-
): Promise<unknown[]>;
13554
ping(callback: SendCallback, data?: unknown): Promise<unknown[]>;
136-
applyIframesApi(api: string): void;
137-
getIframeEl(): HTMLIFrameElement;
138-
getSiteEl(): HTMLElement;
139-
setSiteEl(el: HTMLElement): void;
140-
getWindow(): Window;
141-
getOrigin(): string;
142-
close(params?: Params, callback?: SendCallback): Promise<unknown[]>;
143-
restyle(style: StyleData, callback?: SendCallback): Promise<unknown[]>;
144-
registerWasRestyled<T extends Message>(
145-
handler: MessageHandler<T>,
146-
filter?: IframesFilter
147-
): void;
148-
registerWasClosed<T extends Message>(
149-
handler: MessageHandler<T>,
150-
filter?: IframesFilter
151-
): void;
55+
restyle(style: Record<string, string|boolean>, callback?: SendCallback): Promise<unknown[]>;
15256
}
15357

154-
const SAME_ORIGIN_IFRAMES_FILTER: IframesFilter;
15558
const CROSS_ORIGIN_IFRAMES_FILTER: IframesFilter;
15659

157-
let selfContext: Context;
158-
159-
function create(
160-
url: string,
161-
where: HTMLElement,
162-
options?: OptionsBag
163-
): HTMLIFrameElement;
16460
function getContext(): Context;
165-
function makeWhiteListIframesFilter(origins: string[]): IframesFilter;
166-
function registerStyle(style: string, handler: StyleHandler): void;
167-
function registerBeforeOpenStyle(style: string, handler: StyleHandler): void;
168-
function getStyle(style: string): StyleHandler;
169-
function getBeforeOpenStyle(style: string): StyleHandler;
170-
function registerIframesApi<T extends Message>(
171-
api: string,
172-
registry: { [key: string]: MessageHandler<T> },
173-
filter?: IframesFilter
174-
): void;
175-
function registerIframesApiHandler<T extends Message>(
176-
api: string,
177-
message: string,
178-
handler: MessageHandler<T>
179-
): void;
18061
}

packages-exp/auth-exp/src/platform_browser/iframe/gapi.test.ts

Lines changed: 13 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -25,31 +25,24 @@ import { FirebaseError } from '@firebase/util';
2525
import { testAuth } from '../../../test/mock_auth';
2626
import { Auth } from '../../model/auth';
2727
import { AUTH_WINDOW } from '../auth_window';
28+
import * as js from '../load_js';
2829
import { _loadGapi, _resetLoader } from './gapi';
2930

3031
use(sinonChai);
3132
use(chaiAsPromised);
3233

3334
describe('src/platform_browser/iframe/gapi', () => {
34-
let library: unknown;
35-
let tag: HTMLElement;
35+
let library: typeof gapi;
3636
let auth: Auth;
37-
function onJsLoad(): void {
37+
function onJsLoad(globalLoadFnName: string): void {
3838
AUTH_WINDOW.gapi = library as typeof gapi;
39-
AUTH_WINDOW[(tag as HTMLScriptElement).src.split('onload=')[1]]();
39+
AUTH_WINDOW[globalLoadFnName]();
4040
}
4141

4242
beforeEach(async () => {
43-
const head = document.createElement('div');
44-
tag = document.createElement('script');
45-
46-
sinon.stub(document, 'createElement').returns(tag);
47-
sinon
48-
.stub(document, 'getElementsByTagName')
49-
.returns(([head] as unknown) as HTMLCollection);
50-
sinon.stub(head, 'appendChild').callsFake(() => {
51-
onJsLoad();
52-
return head;
43+
sinon.stub(js, '_loadJS').callsFake(url => {
44+
onJsLoad(url.split('onload=')[1]);
45+
return Promise.resolve(new Event('load'));
5346
});
5447

5548
auth = await testAuth();
@@ -58,7 +51,7 @@ describe('src/platform_browser/iframe/gapi', () => {
5851
function makeGapi(
5952
result: unknown,
6053
timesout = false
61-
): Record<string, unknown> {
54+
): typeof gapi {
6255
const callbackFn = timesout === false ? 'callback' : 'ontimeout';
6356
return {
6457
load: sinon
@@ -67,9 +60,9 @@ describe('src/platform_browser/iframe/gapi', () => {
6760
params[callbackFn]()
6861
),
6962
iframes: {
70-
getContext: () => result
63+
getContext: () => result as gapi.iframes.Context,
7164
}
72-
};
65+
} as unknown as typeof gapi;
7366
}
7467

7568
afterEach(() => {
@@ -117,11 +110,11 @@ describe('src/platform_browser/iframe/gapi', () => {
117110
expect(await _loadGapi(auth)).to.eq('test');
118111
expect(await _loadGapi(auth)).to.eq('test');
119112

120-
expect(document.createElement).to.have.been.calledOnce;
113+
expect(js._loadJS).to.have.been.calledOnce;
121114
});
122115

123116
it('rejects with a network error if load fails', async () => {
124-
library = {};
117+
library = {} as typeof gapi;
125118
await expect(_loadGapi(auth)).to.be.rejectedWith(
126119
FirebaseError,
127120
'auth/network-request-failed'
@@ -137,7 +130,7 @@ describe('src/platform_browser/iframe/gapi', () => {
137130
});
138131

139132
it('resets the load promise if the load errors', async () => {
140-
library = {};
133+
library = {} as typeof gapi;
141134
const firstAttempt = _loadGapi(auth);
142135
await expect(firstAttempt).to.be.rejectedWith(
143136
FirebaseError,

packages-exp/auth-exp/src/platform_browser/iframe/gapi.ts

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ import { AUTH_ERROR_FACTORY, AuthErrorCode } from '../../core/errors';
1919
import { Delay } from '../../core/util/delay';
2020
import { Auth } from '../../model/auth';
2121
import { AUTH_WINDOW } from '../auth_window';
22-
import { _generateCallbackName, _loadJS } from '../load_js';
22+
import * as js from '../load_js';
2323

2424
const NETWORK_TIMEOUT = new Delay(30000, 60000);
2525
const LOADJS_CALLBACK_PREFIX = 'iframefcb';
@@ -32,7 +32,6 @@ function resetUnloadedGapiModules(): void {
3232
// Clear last failed gapi.load state to force next gapi.load to first
3333
// load the failed gapi.iframes module.
3434
// Get gapix.beacon context.
35-
console.warn(AUTH_WINDOW.___jsl);
3635
const beacon = AUTH_WINDOW.___jsl;
3736
// Get current hint.
3837
if (beacon?.H) {
@@ -88,7 +87,6 @@ function loadGapi(auth: Auth): Promise<gapi.iframes.Context> {
8887
// If gapi.iframes.Iframe available, resolve.
8988
resolve(gapi.iframes.getContext());
9089
} else if (!!AUTH_WINDOW.gapi?.load) {
91-
console.error('here');
9290
// Gapi loader ready, load gapi.iframes.
9391
loadGapiIframe();
9492
} else {
@@ -97,7 +95,7 @@ function loadGapi(auth: Auth): Promise<gapi.iframes.Context> {
9795
// multiple times in parallel and could result in the later callback
9896
// overwriting the previous one. This would end up with a iframe
9997
// timeout.
100-
const cbName = _generateCallbackName(LOADJS_CALLBACK_PREFIX);
98+
const cbName = js._generateCallbackName(LOADJS_CALLBACK_PREFIX);
10199
// GApi loader not available, dynamically load platform.js.
102100
AUTH_WINDOW[cbName] = () => {
103101
// GApi loader should be ready.
@@ -113,7 +111,7 @@ function loadGapi(auth: Auth): Promise<gapi.iframes.Context> {
113111
}
114112
};
115113
// Load GApi loader.
116-
return _loadJS(`https://apis.google.com/js/api.js?onload=${cbName}`);
114+
return js._loadJS(`https://apis.google.com/js/api.js?onload=${cbName}`);
117115
}
118116
}).catch(error => {
119117
// Reset cached promise to allow for retrial.

packages-exp/auth-exp/src/platform_browser/iframe/iframe.test.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,6 @@ describe('src/platform_browser/iframe/iframe', () => {
6868

6969
it('sets all the correct settings', async () => {
7070
await _openIframe(auth);
71-
console.warn(iframeSettings);
7271

7372
expect(iframeSettings.where).to.eql(document.body);
7473
expect(iframeSettings.url).to.eq(

packages-exp/auth-exp/src/platform_browser/iframe/iframe.ts

Lines changed: 6 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -65,15 +65,14 @@ export async function _openIframe(auth: Auth): Promise<gapi.iframes.Iframe> {
6565
// Prevent iframe from closing on mouse out.
6666
setHideOnLeave: false
6767
});
68+
69+
const networkError = AUTH_ERROR_FACTORY.create(AuthErrorCode.NETWORK_REQUEST_FAILED, {
70+
appName: auth.name
71+
});
6872
// Confirm iframe is correctly loaded.
6973
// To fallback on failure, set a timeout.
70-
console.warn('setting timeout');
7174
const networkErrorTimer = AUTH_WINDOW.setTimeout(() => {
72-
reject(
73-
AUTH_ERROR_FACTORY.create(AuthErrorCode.NETWORK_REQUEST_FAILED, {
74-
appName: auth.name
75-
})
76-
);
75+
reject(networkError);
7776
}, PING_TIMEOUT.get());
7877
// Clear timer and resolve pending iframe ready promise.
7978
function clearTimerAndResolve(): void {
@@ -83,11 +82,7 @@ export async function _openIframe(auth: Auth): Promise<gapi.iframes.Iframe> {
8382
// This returns an IThenable. However the reject part does not call
8483
// when the iframe is not loaded.
8584
iframe.ping(clearTimerAndResolve).then(clearTimerAndResolve, () => {
86-
reject(
87-
AUTH_ERROR_FACTORY.create(AuthErrorCode.NETWORK_REQUEST_FAILED, {
88-
appName: auth.name
89-
})
90-
);
85+
reject(networkError);
9186
});
9287
})
9388
);

0 commit comments

Comments
 (0)