[Bug]:@use-funnel/browser is not being processed on the server side.

[XionWCFM]

Package Scope

@use-funnel/browser

Bug description

I think this is a bug.

@use-funnel's docs recommend using @use-funnel/browser when using the next.js approuter, but

In fact, if you use @use-funnel/browser in the app router, the project cannot be built because server-side processing is not performed properly.

This issue can also be found in @use-funnel/browser@0.0.5 version.

The reason this problem occurs is that using the 'use client' directive when using the next.js app router does not mean that the file will never be evaluated on the server side.

In fact, next.js also analyzes files containing the use client directive during the build process, and if there is code that accesses the window object, an error occurs.

If we want to pass the use case for app router to @use-funnel/browser, I think we will have to do server-side processing.

Expected behavior

We want error-free builds.

To Reproduce

@use-funnel/browser 0.0.5
next 14.2.13

Possible Solution

I made a few modifications to this code base locally to resolve this issue.

Simply put, the problem of errors occurring during build can be solved by modifying the code so that it does not directly access the window object.

Because this is exactly the code that is causing the problem.

export const useFunnel = createUseFunnel(({ id, initialState }) => {
  const [location, setLocation] = useState(() => ({
    search: window.location.search,
  }));
  const [state, setState] = useState(() => ({
    ...window.history.state,
  }));

This code raises an error in an environment where the window object does not exist.

So, you can modify it like this:

  const [location, setLocation] = useState({ search: '' });
  const [state, setState] = useState<any>({});

  useEffect(() => {
    if (typeof window !== 'undefined') {
      setLocation({ search: window.location?.search ?? '' });
      setState(window.history?.state ?? {});

      const handlePopState = (event: PopStateEvent) => {
        setLocation({ search: window.location?.search ?? '' });
        setState(event?.state ?? {});
      };

      window.addEventListener('popstate', handlePopState);
      return () => {
        window.removeEventListener('popstate', handlePopState);
      };
    }
  }, []);

This code is buildable because it does not access the window object on the server side and only reflects the state through useEffect on the client side, and can also avoid the Text content does not match server-rendered HTML error.

However, there are user experience issues with this approach.

Since the initial value of the server is first rendered and updated according to the current position through useEffect, if it is not initialStep, initiailStep is shown first and then the UI blinks as the desired step is rendered.

What's even more disappointing is that this approach has the potential to affect use cases that don't use app-router.

So I think there are two options.

1. Create a new adapter for app-router, place a dependency on next, and create a package that uses hooks provided by next.js, such as useSearchParams.

2. Modify the @use-funnel/browser package to take server side into consideration.

In my opinion, if there is a way to solve the problem of UI flickering when @use-funnel/browser properly supports the server-side environment while also properly supporting the server-side environment, that would be the best method.

Alternatively, creating an adapter for app-router seems attractive.

Do you have any ideas?

etc.

Or there is another way: Using dynamic in next.js ensures that it will always run only on the client, thus solving UI flickering issues and build issues.

However, this method requires more knowledge from the user.

import dynamic from "next/dynamic";
const Example = dynamic(() => import('../src/funnel'))

thank you!

[minuukang]

Wow! I thought when using 'use client' directive, should window.* commands not evaluated at server side. but it is incorrect.

I think we should add isServer condition, and using like this.

const isServer = typeof window === 'undefined';
const [location, setLocation] = useState(() => ({
  search: isServer ? '' : window.location.search,
}));
const [state, setState] = useState(() => ({
  ...isServer ? {} : window.history.state,
}));

This can be not rerender at first render compared by useEffect solution.

[XionWCFM]

That solution creates a hydration error in non-initial cases.

This is related to the Text content does not match server-rendered HTML error I mentioned above.

The server always returns the initial value, but the render actually attempted by the client produces different results depending on the window object, so the results of server rendering and client rendering are different.

Therefore, to avoid hydration errors, we had to handle side effects in useEffect.

However, if you tolerate hydration errors, this approach was one of the solutions to avoid UI flickering.

What do you think?

[minuukang]

oh i doesn't understand about next.js app router build and 'use client'.

You solution is right. it should be inside in effect by only using client.

[minuukang]

I will use useLayoutEffect to solve this solution to avoid flikering!

[XionWCFM]

I will use useLayoutEffect to solve this solution to avoid flikering!

I also thought of that method and tried using layoutEffect, but the flickering was still there.

It is difficult to explain the reasons for this, but the phenomenon can be identified.

Test it in your app router using the simple code snippet below:

export default function Home() {
  const state = useExample()
  return (
    <div>
      {state}
    </div>
  );
}


const useExample = () => {
  const [state,setState] = useState('default state')
  useLayoutEffect(() => {
    setState('layout state')
  } , [])
  return state
}

If you run this code in the app-router environment, the layout state will blink after the default state.

When I tested it, useFunnel was the same.

The only reliable way to eliminate flickering that I have found so far is using useState(() => ) with next dynamic

Server-side processing seems difficult. What do you think?

[minuukang]

Thats right. it should be add documention with using @use-funnel/browser and next.js app router, or add a new package about @use-funnel/next-app-router

[XionWCFM]

Personally, I think it would be a good idea to add documentation until the app-router-adapter is ready.

Even if you do not change the current implementation of @use-funnel/browser, if you import a component that uses useFunnel through dynamic on the user side, you will be free from flickering problems and server-side build problems.

Perhaps the app router avoids this flickering problem by triggering Suspense until useSearchParams is ready.

So I haven't tried it yet, but creating an app router adapter seems like a faster solution than fixing @use-funnel/browser.

I would like to see if there is anything I can do to solve this problem.

If you have any plans to create app-router-adapter, I would like to participate.

[minuukang]

If we want to add new packages, we need to be able to add an array of funnel states to history.state. However, we need to find a way to access history.state in the next.js app router safely from the server build.

Like: https://github.com/toss/use-funnel/blob/main/packages/browser/src/index.ts#L36-L40

And you can test add adapter package, follow this link

[minuukang]

Or, your solution of #63 can be a good idea (using suspense about ready for using history.state)

[XionWCFM]

Or, your solution of #63 can be a good idea (using suspense about ready for using history.state)

Unfortunately, the suggestion in #63 only works with pages routers.

This is because the app router and pages router use different router instances.

If using history.state is so important we should probably create our own.

An implementation like 'useHistory' that triggers suspense instead of producing a blink without causing a hydration missmatch.

---

I've tried several simple methods, but I haven't found a way to implement all of these requirements yet.

next.js's useSearchParams seemed to take the approach of checking whether the above requirements are a server by checking whether a specific instance within next.js exists only on the server, and then skipping pre-rendering by raising an error if it is a server.

next.js useSearchParams implementation | bailout-to-client-rendering.ts

adopt this approach, i can skip rendering on the server and always access the window object on the client.

Instead, there was no way to ignore server errors.

Maybe I'm just stuck with the next.js implementation and not being imaginative.

Do you have any ideas?

[minuukang]

After discussing the refresh implementation of @use-funnel/next, we are considering supporting both history.state and querystrings at the same time. This is likely to be implemented similarly for @use-funnel/next-app-router in the future.

Currently, @use-funnel/next has the funnel state history in history.state(in next.js page router, using asPath trick) and an index to the current history in a query string. However, there is an issue with losing the funnel history after a refresh.

We are looking to improve this to store the current step and context in a query string, and only store the "old" history in history.state (#69). Some features (overlays) that use the old history will be unavailable on refresh, but at least we can guarantee the state for the current history.

After some more testing, I'm going to merge that PR. Feedback on that PR is also welcome!

[XionWCFM]

의 새로 고침 구현을 논의한 후 @use-funnel/next, 우리는 history.state동시에 와 쿼리 문자열을 모두 지원하는 것을 고려하고 있습니다. 이는 @use-funnel/next-app-router미래에도 비슷하게 구현될 가능성이 높습니다.

현재, (next.js 페이지 라우터에서 트릭을 사용하여 ) @use-funnel/next퍼널 상태 기록이 있고 쿼리 문자열에 현재 기록에 대한 인덱스가 있습니다. 그러나 새로 고침 후 퍼널 스태프 기록을 잃는 문제가 있습니다.history.state``asPath

우리는 이것을 개선하여 현재 직원과 컨텍스트를 쿼리 문자열에 저장하고, "이전" 기록만 history.state( #69 )에 저장하려고 합니다. 이전 기록을 사용하는 일부 기능(오버레이)은 새로 고침 시 사용할 수 없지만, 최소한 현재 기록의 상태는 보장할 수 있습니다.

몇 가지 테스트를 더 거친 후, 그 PR을 병합할 겁니다. 그 PR에 대한 피드백도 환영합니다!

@use-funnel/next I understand what trick it was implemented with.

So it was different from what you wanted.

I think storing the context in the query string is a really good idea.

I'm looking forward to the next version of @use-funnel