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

Proposal: dynamic search namespace #1303

Closed
palkan opened this issue Jul 23, 2020 · 6 comments · Fixed by #1307
Closed

Proposal: dynamic search namespace #1303

palkan opened this issue Jul 23, 2020 · 6 comments · Fixed by #1307

Comments

@palkan
Copy link
Contributor

palkan commented Jul 23, 2020

Feature request

What problem does this feature solve?

Make it possible to dynamically change search namespace to isolate different parts of the documentation.
For example, we can isolate different locales from each other while using aliases. Right now, search index is built for all pages, and hence some queries return results for multiple locales at once (try searching for index.html at https://docsify.js.org/).

What does the proposed API look like?

I'm currently using a patched version of the search plugin which updates CONFIG.namespace in afterEach callback by using the value from vm.config.search.namespace: https://github.com/test-prof/docs/blob/cb7826d37b37a338df64830fa85f6132f82992a6/docs/assets/docsify-search.js#L325

Thus, in order to change the namespace you need to update vm.config.search.namespace in your plugin.

That works fine for me but it's not an ideal solution.

Probably, exposing an API to switch the namespace would be better, e.g.: vm.config.search.switchNamespace(...) or even vm.plugins.search.switchNamespace(...).

How should this be implemented in your opinion?

Long term: provide and API to access plugin instances, and thus making it possible to communicate with plugins from other plugins.

Short term: update namespace from the current vm.config settings in the init function:

export function init(config, vm) {

Are you willing to work on this yourself?

Sure.

@sy-records
Copy link
Member

Maybe fixed by #1300?

you can try it in https://docsify-preview-git-fork-sy-records-fix-search.docsify-core.vercel.app

You can also submit pr if you have a better way.

@anikethsaha
Copy link
Member

As mentioned above, if this is similar to 1300, would be great.

about the long term goal, I believe it would be good to have shared data instead of directly communicating with the other plugins from a plugin

WDYT ?

@palkan
Copy link
Contributor Author

palkan commented Jul 23, 2020

Maybe fixed by #1300?

Yeah, that fix solves the issue but requires reindexing every time a user switch locale to a non-default one ('cause localStorage only contains an index for a single namespace).

I got a new idea: prefixes. Here is a sketch of idea:

// configuration
search: {
  paths: 'auto',
  // prefixes could be multipart
  prefixes: ['/ru-ru', '/zh-cn', '/ru-ru/v1'] 
}

init(config, vm) {
  // first, get paths
  const paths = isAuto ? getAllPaths(vm.router) : config.paths;

  let namespaceSuffix = '';

  // only in auto mode
  if (isAuto) {
    const path = paths[0];
    const prefix = config.prefixes.find(prefix => path.startsWith(prefix);

    if (prefix) namespaceSuffix = prefix;
  }

  const expireKey = resolveExpireKey(config.namespace) + namespaceSuffix;
  const indexKey = resolveIndexKey(config.namespace) + namespaceSuffix;

  // current implementation
}

This way we maintain multiple indexes and keep them independently in local storage.

WDYT?

@anikethsaha
Copy link
Member

It looks like nonbreaking. (correct me if I am wrong)

seems good for versioning (#1289) as well.

+1

cc @docsifyjs/reviewers

@palkan
Copy link
Contributor Author

palkan commented Jul 23, 2020

It looks like nonbreaking. (correct me if I am wrong)

Yep, it is.

@Koooooo-7
Copy link
Member

Cool.
@sy-records and I discussed about the namespace for solving #1300 issue , but we hvnt find an ideal way yet.
Glad to see the draft and wait for the PR. 👍

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

Successfully merging a pull request may close this issue.

4 participants