tajo / ladle

🥄 Develop, test and document your React story components faster.
https://www.ladle.dev
MIT License
2.63k stars 93 forks source link

Allow for user defined query parameters #496

Closed getinnocuous closed 1 year ago

getinnocuous commented 1 year ago

Is your feature request related to a problem? Please describe. Currently whenever you add any custom query parameters, they get removed by Ladle. For example, opening a modal via a query param of "?modalOpen=1"

Describe the solution you'd like It would be great to allow custom query parameters per story that would be present on initial load of the story, as well as being able to update/add/remove parameters

Describe alternatives you've considered I've tried adding the query parameters via a useEffect in the components.tsx like below but it doesn't work.

useEffect(() => {
  if (queryParams) {
    const urlParams = new URLSearchParams(window.location.search)
    Object.entries(queryParams).forEach((e) => {
      urlParams.set(e[0], e[1])
    })
  }
}, [queryParams])
tajo commented 1 year ago

This could be pretty straightforward to implement - Ladle should preserve existing non-ladle params.

Relevant code: https://github.com/tajo/ladle/blob/main/packages/ladle/lib/app/src/history.ts#L25

cm-dyoshikawa commented 1 year ago

Could I work on it?

tajo commented 1 year ago

Of course!

cm-dyoshikawa commented 1 year ago

Thanks. I will research it.

cm-dyoshikawa commented 1 year ago

@tajo

Probably what @getinnocuous wants is a mechanism that allows setting query parameters for each Story. If implemented, it could make the cataloging of components that depend on the value of the query parameter more effective.

There exists an Addon in Storybook that realizes it. So, how about adding an Addon that can set query parameters in Ladle as well?

It will provide the following API:

import { Story } from "@ladle/react";

export const Hello: Story = () => {
  const urlSearchParams = new URLSearchParams(document.location.search);
  return (
    <>
      <h1>Hello Query Parameters</h1>
      <p>{urlSearchParams.get("foo")}</p>
    </>
  );
};

Hello.meta = {
  // Set query params
  query: {
    foo: "bar",
  },
};
tajo commented 1 year ago

I think @getinnocuous just wants to be able to programmatically set query params (aka their stories are reading/setting query params). Ladle now strips them away on any action - it should preserve them unless they collide with Ladle's params.

The add-on you describe is a different use-case. Not sure if its that useful since you can just use useEffect() to initialize query params.

cm-dyoshikawa commented 1 year ago

OK. I considering.

cm-dyoshikawa commented 1 year ago

Changing the code in the following way seems to make the combination of useEffect and history.pushState work

export const modifyParams = (globalState: GlobalState) => {
  if (!globalState.controlInitialized) return;
  const params = {
+   ...queryString.parse(location.search),
    mode: globalState.mode,
    rtl: globalState.rtl,
    source: globalState.source,
    story: globalState.story,
    theme: globalState.theme,
    width: globalState.width,
    control: globalState.control,
  };

Sample code for using the library:

import type { Story } from "@ladle/react";
import { useEffect, useState } from "react";

export const CardTest: Story = () => {
  const [queryParams, setQueryParams] = useState<string>("");

  useEffect(() => {
    setQueryParams(location.search);
  }, []);

  return <p>Params: {queryParams}</p>;
  // Expected => "Params: ?story=query-parameters--card-test&foo=bar"
  // Actual => "Params: ?story=query-parameters--card-test"
};

CardTest.decorators = [
  (Component) => {
    useEffect(() => {
      const url = new URL(window.location.href);
      url.searchParams.set("foo", "bar");
      history.pushState({}, "", url);
    }, []);

    return (
      <>
        <Component />
      </>
    );
  },
];

However, in this case as well, it seems challenging to capture the URL query parameters after the component has changed. Most React components that use query parameters are likely expected to obtain their value at the initial rendering with useEffect(() => {}, []). But, there are cases where these changes may not be ready at the time of this acquisition.

I want your opinion.

tajo commented 1 year ago
useEffect(() => {
  setQueryParams(location.search);
}, [location.search]);

would fix that?

cm-dyoshikawa commented 1 year ago

Good point, thank you. I'll create the PR.

tajo commented 1 year ago

landed as part of v3.2, thanks to @cm-dyoshikawa

getinnocuous commented 1 year ago

legends! great work