Skip to content
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

Fixed assert about changing Pretender config. Ensured trackRequest was set properly #1045

Merged
merged 2 commits into from
Jun 1, 2022

Conversation

cah-brian-gantzler
Copy link
Collaborator

@cah-brian-gantzler cah-brian-gantzler commented Jun 1, 2022

Fixed the assert that informed you could not change the trackRequests after pretender is created.

After moving things around, the value of tracked Request was being sent into pretender via a function. The value in the function was being set too late. Since the value was already known, just simplified as putting it behind a function served no purpose.

Fixes #1043

@cah-brian-gantzler
Copy link
Collaborator Author

Seems there is a test that actually checks that the assert is thrown. Rather than remove the assert, will see if code can be restructured to still make the assert possible

…anging tracked requests after pretender is created
@cah-brian-gantzler
Copy link
Collaborator Author

I have made these same changes in mirage-pretender. Will release a version of that once this is merged and confirmed to fix the issue.

Copy link
Collaborator

@IanVS IanVS left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for hopping into this one.

@@ -129,7 +131,8 @@ export default class PretenderConfig {
}

let didOverridePretenderConfig =
config.trackRequests !== undefined && this.pretender;
config.trackRequests !== undefined &&
config.trackRequests !== this.trackRequests;
Copy link
Collaborator

Choose a reason for hiding this comment

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

So, instead of checking whether there's a pretender instance, we look to see whether the trackRequests has changed. This makes sense, although the name of didOverridePretenderConfig feels overly broad for what's actually being checked. But, I think that's fine.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Its a local var, I was just changing the minimum

@IanVS IanVS merged commit bf4a73e into miragejs:master Jun 1, 2022
@IanVS IanVS changed the title Removed assert about changing Pretender config. Ensured trackRequest was set properly Fixed assert about changing Pretender config. Ensured trackRequest was set properly Jun 1, 2022
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.

I simply can't use/config trackRequests anymore and I'm not sure why
2 participants