Skip to content

Conversation

@semanticart
Copy link
Collaborator

This is most relevant to react tests since there's a single global
prefab object, but if you init after starting polling, the previous
polling shouldn't continue.

This is most relevant to react tests since there's a single global
`prefab` object, but if you `init` after starting polling, the previous
polling shouldn't continue.
Comment on lines +139 to +141
if (!prefab.loader) {
throw new Error('Expected loader to be set');
}
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.

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

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

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.

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

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');

@semanticart semanticart merged commit c0ccb95 into main Jul 11, 2023
@semanticart semanticart deleted the prevent-multi-polling branch July 11, 2023 12:28
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 this pull request may close these issues.

3 participants