Skip to content

Reset polling on init #21

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.

Sign up for GitHub

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

Merged
semanticart merged 2 commits into from
Jul 11, 2023
Merged

Reset polling on init #21

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c1e75ee
Reset polling on `init`
semanticart Jul 10, 2023
1da020b
Fix duplicate prettier
semanticart Jul 11, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
"jest": "^28.1.3",
"jest-fetch-mock": "^3.0.3",
"lint-staged": "^13.2.3",
"prettier": "^2.8.8",
"prettier": "^3.0.0",
"ts-node": "^10.9.1",
"typescript": "^4.7.4"
Expand Down
41 changes: 40 additions & 1 deletion src/prefab.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,11 @@ beforeEach(() => {
prefab.configs = {};
});

afterEach(() => {
prefab.stopPolling();
prefab.pollCount = 0;
});

describe('init', () => {
it('works when the request is successful', async () => {
const data = {values: {turbo: {double: 2.5}}};
Expand Down Expand Up @@ -91,7 +96,6 @@ describe('poll', () => {

await prefab.poll({frequencyInMs});
expect(prefab.loader.context).toStrictEqual(prefab.context);
await wait(1);

if (prefab.pollStatus.status !== 'running') {
throw new Error('Expected pollStatus to be running');
Expand All @@ -117,6 +121,41 @@ describe('poll', () => {
await wait(frequencyInMs * 2);
expect(prefab.pollCount).toEqual(2);
});

it('is reset on init', async () => {
jest.spyOn(globalThis, 'clearTimeout');

const data = {values: {}};
const frequencyInMs = 25;
fetchMock.mockResponse(JSON.stringify(data));

const config: InitParams = {
apiKey: '1234',
context: new Context({user: {device: 'desktop'}}),
};

await prefab.init(config);

if (!prefab.loader) {
throw new Error('Expected loader to be set');
}
Comment on lines +139 to +141
Copy link
Contributor

@mjfaga mjfaga Jul 10, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

You may want to write these as expectations instead?

Suggested change
if (!prefab.loader) {
throw new Error('Expected loader to be set');
}
expect(prefab.loader).not.toBeNull();

Copy link
Collaborator Author

@semanticart semanticart Jul 10, 2023

Choose a reason for hiding this comment

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

A reasonable thing to suggest. But if I do that, I still have to guard against this being undefined or when I later call prefab.loader.context typescript complains error TS18048: 'prefab.loader' is possibly 'undefined'.

There are other ways to handle this, but IME this is the simplest way to handle it in test code.

If you have a succinct preferred approach, please do tell!

Copy link
Contributor

@mjfaga mjfaga Jul 10, 2023

Choose a reason for hiding this comment

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

Ahhhh, good ol' typescript doesn't pick up the guards from the expectations in jest yet without writing wrapper functions per this PR: microsoft/TypeScript#32695

👍 keep on trucking.


await prefab.poll({frequencyInMs});
expect(prefab.loader.context).toStrictEqual(prefab.context);

if (prefab.pollStatus.status !== 'running') {
throw new Error('Expected pollStatus to be running');
}
Comment on lines +146 to +148
Copy link
Contributor

@mjfaga mjfaga Jul 10, 2023

Choose a reason for hiding this comment

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

Suggested change
if (prefab.pollStatus.status !== 'running') {
throw new Error('Expected pollStatus to be running');
}
expect(prefab.pollStatus.status).toEqual('running');

expect(prefab.pollCount).toEqual(0);
expect(prefab.loader.context).toStrictEqual(prefab.context);

const timeoutId = prefab.pollTimeoutId;

await prefab.init(config);
expect(prefab.pollStatus).toEqual({status: 'stopped'});
expect(clearTimeout).toHaveBeenCalledWith(timeoutId);
expect(prefab.pollTimeoutId).toBeUndefined();
});
});

describe('setConfig', () => {
Expand Down
20 changes: 11 additions & 9 deletions src/prefab.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,7 @@ type PollStatus =
| {status: 'not-started'}
| {status: 'pending'}
| {status: 'stopped'}
| {
status: 'running';
frequencyInMs: number;
timeoutId: ReturnType<typeof setTimeout>;
};
| {status: 'running'; frequencyInMs: number};

export const prefab = {
configs: {} as {[key: string]: Config},
Expand All @@ -38,6 +34,8 @@ export const prefab = {

pollCount: 0,

pollTimeoutId: undefined as ReturnType<typeof setTimeout> | undefined,

async init({
apiKey,
context: providedContext,
Expand All @@ -51,6 +49,10 @@ export const prefab = {
throw new Error('Context must be provided');
}

if (this.pollTimeoutId) {
this.stopPolling();
}

this.context = context;

this.loader = new Loader({
Expand Down Expand Up @@ -82,8 +84,9 @@ export const prefab = {
},

stopPolling() {
if (this.pollStatus.status === 'running') {
clearInterval(this.pollStatus.timeoutId);
if (this.pollTimeoutId) {
clearTimeout(this.pollTimeoutId);
this.pollTimeoutId = undefined;
}

this.pollStatus = {status: 'stopped'};
Expand Down Expand Up @@ -128,7 +131,7 @@ async function load() {
}

async function doPolling({frequencyInMs}: {frequencyInMs: number}) {
const pollTimeoutId = setTimeout(() => {
prefab.pollTimeoutId = setTimeout(() => {
load().finally(() => {
if (prefab.pollStatus.status === 'running') {
doPolling({frequencyInMs});
Expand All @@ -139,6 +142,5 @@ async function doPolling({frequencyInMs}: {frequencyInMs: number}) {
prefab.pollStatus = {
status: 'running',
frequencyInMs,
timeoutId: pollTimeoutId,
};
}