Skip to content

feat: Added initialRoute parameter to avoid resolver issues with using a default route #367

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

timdeschryver merged 2 commits into testing-library:main from JJosephttg:initial-render-option

Feb 18, 2023

Merged

feat: Added initialRoute parameter to avoid resolver issues with using a default route #367

timdeschryver merged 2 commits into testing-library:main from JJosephttg:initial-render-option

Feb 18, 2023

Conversation

Copy link

Contributor

@JJosephttg JJosephttg commented Feb 12, 2023

Added the ability to specify an initial route to render with. As someone who wanted to use my original routes in my tests, I found that even though I do initial navigation without detecting changes in my tests using navigate, I would still run into issues where my resolver would get triggered despite the fact I was switching before detecting changes.

This allows navigation to happen before the location listener is set up so that you may perform an initial route navigation without causing any resolvers/guards to run and additionally having to specify await navigate(...) in your tests


 Added initialRoute param to render

3f1c5cf

timdeschryver reviewed

View reviewed changes

Copy link

Member

@timdeschryver timdeschryver left a comment

Choose a reason for hiding this comment

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

Thanks, I like it!

projects/testing-library/tests/render.spec.ts Outdated

}

it('allows initially rendering a specific route to avoid triggering a resolver for the default route', async () => {

const expectedRoute = 'correct-route';

Copy link

Member

@timdeschryver timdeschryver Feb 17, 2023

Choose a reason for hiding this comment

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

Could you rename this to the following please.
I do think it makes it intention clearer.

Suggested change

const expectedRoute = 'correct-route';

const initialRoute = 'initial-route';

projects/testing-library/tests/render.spec.ts Outdated

expect(resolver.isResolved).toBe(false);

expect(screen.queryByText('Secondary Component')).not.toBeInTheDocument();

expect(await screen.findByText('button')).toBeInTheDocument();

Copy link

Member

@timdeschryver timdeschryver Feb 17, 2023

Choose a reason for hiding this comment

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

There's no need to await here.

Suggested change

expect(awaitscreen.findByText('button')).toBeInTheDocument();

expect(screen.getByText('button')).toBeInTheDocument();

projects/testing-library/tests/render.spec.ts Outdated

await render(RouterFixtureComponent, {

initialRoute: expectedRoute,

routes,

detectChangesOnRender: false,

Copy link

Member

@timdeschryver timdeschryver Feb 17, 2023

Choose a reason for hiding this comment

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

Is there a reason why detect changes is set to false?

Suggested change

detectChangesOnRender: false,

Copy link

Contributor Author

@JJosephttg JJosephttg Feb 18, 2023

Choose a reason for hiding this comment

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

Mainly just to isolate the test from any extra logic, but I can remove it since it doesn't add any value here


 Corrected tests based on suggestions

f43bbbb

Copy link

Contributor Author

JJosephttg commented Feb 18, 2023

Just updated the tests based on your suggestions!

timdeschryver approved these changes

View reviewed changes

@timdeschryver timdeschryver merged commit d3407ca into testing-library:main

Copy link

Member

timdeschryver commented Feb 18, 2023

@all-contributors please add @JJosephttg for code, tests

@allcontributors

Copy link

Contributor

allcontributors bot commented Feb 18, 2023

@JJosephttg already contributed before to code, test

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants

@JJosephttg @timdeschryver