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

feat: Make isSearchPath a callback parameter #74

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jdeniau
Copy link
Contributor

@jdeniau jdeniau commented Mar 18, 2022

Force /recherche and /search pages to be search pages seems really weird, and probably and internal application need.

This PR opens the discussion about changing this behavior.

I just created a isSearchPath callback parameter that keeps your previous behaviour to avoid BC break, but I think that the proper way to do this is to drop a major tag with an empty implementation (path: string) => false, or even to have a searchPath parameters :

searchPaths: [
  '/search.html', // for exact match
  /\/recherche/ // for a pattern matching that can replace your `startWith` call
]

@jdeniau jdeniau changed the title Make isSearchPath a callback parameter feat: Make isSearchPath a callback parameter Mar 18, 2022
@revolunet
Copy link
Member

Thanks for your help on this !

What about releasing a non breaking change with an additional, optional searchPaths parameter that defaults to these values ?

@jdeniau
Copy link
Contributor Author

jdeniau commented Apr 12, 2022

I tried to do so, but I found another issue : the query param q is extracted from the route, and should be a parameter too.

So I think the callback may be a better option and could return the search query, but the implementation might be complex :

const defaultIsSearchPath = (path: string): string | undefined => {
  const matches = path.match(/\?(.*)/);
  if (!matches) {
    return;
  }

  const searchParams = new URLSearchParams(matches )[1]
  if (startsWith(path, "/recherche") || startsWith(path, "/search")) {
    return searchParams.get('q');
  }
}

Or if we extract the query parameters for the callback

const defaultIsSearchPath = (path: string, searchParams: URLSearchParams): string | undefined => {
  if (startsWith(path, "/recherche") || startsWith(path, "/search")) {
    return searchParams.get('q');
  }
}

But I do not really like either implementations. What do you think ?

@razzeee
Copy link

razzeee commented Apr 29, 2024

This would be helpful, as our search page is under /apps/search

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.

3 participants