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

Parameter not read from route #1580

Open
JamesNK opened this issue Oct 10, 2024 · 4 comments · May be fixed by #1584
Open

Parameter not read from route #1580

JamesNK opened this issue Oct 10, 2024 · 4 comments · May be fixed by #1584
Labels
investigate This issue require further investigation before closing. needs design More design is needed before this issue should result in a feature being implemented.

Comments

@JamesNK
Copy link

JamesNK commented Oct 10, 2024

Describe the bug

I have a page that I'm testing with some routes:

@page "/metrics"
@page "/metrics/resource/{ApplicationName}"

There are also some query string parameters:

[Parameter]
public string? ApplicationName { get; set; }

[Parameter]
[SupplyParameterFromQuery(Name = "meter")]
public string? MeterName { get; set; }

I have set a URL in navigation manager:

var navigationManager = Services.GetRequiredService<NavigationManager>();
navigationManager.NavigateTo("/metrics/resource/TestApp?meter=foo");

When the component is rendered I see the MeterName parameter has a value, but the ApplicationName is blank.

Now, I can fix that by manually setting the parameter value when calling RenderComponent:

var cut = RenderComponent<Metrics>(builder =>
{
    builder.Add(m => m.ApplicationName, "TestApp");
});

But part of the code I'm testing changes the current page with NavigateTo, and I need ApplicationName parameter to pick up its new value when navigation happens.

Why isn't ApplicationName parameter value getting its value from the @page "/metrics/resource/{ApplicationName}" page route?

@linkdotnet
Copy link
Collaborator

Hey @JamesNK,

the short answer on why it doesn't get the parameter value from the attribute is because there is no Router involved in bUnit, and the way you did this is (currently) more or less the intended way.

The long answer is here: https://bunit.dev/docs/providing-input/passing-parameters-to-components.html?q=SupplyParameterFromQuery&tabs=csharp#passing-query-parameters-supplyparameterfromquery-to-a-component

Since net8.0, the ParameterAttribute isn't mandatory anymore when SupplyParameterFromQueryAttribute is used, as itself inherits from CascasdingParameterAttribute.

This whole cascade is handled by the NavigationManager. But your ApplicationName isn't - it is resolved by the router, which bUnit doesn't have at all.

Therefore, as you provided the ParameterAttribute on MeterName you should be able to do the following:

var cut = RenderComponent<Metrics>(builder =>
{
    builder.Add(m => m.ApplicationName, "TestApp");
    builder.Add(m => m.MeterName, "Some Value");
});

If you drop the ParameterAttribute then you already do what you "should" do.

The reasoning behind is that the whole @page setup is a 3rd party concern from the point of view of your code, and falls under the same category as JavaScript or CSS stuff (which are also left out in a bUnit test).

Maybe @egil and I can pick this up tomorrow and discuss this further. At least from a documentation point of view, there is room for improvements.

@JamesNK
Copy link
Author

JamesNK commented Oct 10, 2024

Ok. This is what I added to make my test pass:

        navigationManager.LocationChanged += (sender, e) =>
        {
            cut.SetParametersAndRender(builder =>
            {
                builder.Add(m => m.ApplicationName, "TestApp2");
            });
        };

It's surprising that some parameters are automatically bound from navigation manager and others aren't.

@egil
Copy link
Member

egil commented Oct 10, 2024

@JamesNK, I think it is a decent workaround to emulate NavigationManager's behavior in production like that.

Do I understand the scenario correctly: A component under test uses NavigationManager to change the URL. In production, this would cause the component's (page) life cycle methods (OnParameterSet, etc.) to be triggered, and parameters defined in the URL updated on the component under test.

In a bUnit test, only SupplyParameterFromQuery parameters are updated, not "path" parameters?

It's surprising that some parameters are automatically bound from navigation manager and others aren't.

It is, and I dislike that. Prefer not to surprise users. As far as I can tell (or remember), our FakeNavigationManager does not do anything to support automatically updating SupplyParameterFromQuery parameters when its NavigateTo methods are called, so I am guessing that the core Blazor renderer does this for us.

I think it should be possible to enable this for path parameters too, since our renderer could subscribe to LocationChanged and trigger a re-render of any components it has rendered if they have a PageAttribute with path parameters. It would probably be a breaking change for v1 users if enabled by default though.

@JamesNK
Copy link
Author

JamesNK commented Oct 10, 2024

Do I understand the scenario correctly: A component under test uses NavigationManager to change the URL. In production, this would cause the component's (page) life cycle methods (OnParameterSet, etc.) to be triggered, and parameters defined in the URL updated on the component under test.

Yes

@linkdotnet linkdotnet added investigate This issue require further investigation before closing. needs design More design is needed before this issue should result in a feature being implemented. labels Oct 11, 2024
@linkdotnet linkdotnet linked a pull request Oct 12, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
investigate This issue require further investigation before closing. needs design More design is needed before this issue should result in a feature being implemented.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants