Skip to content
This repository has been archived by the owner on Apr 11, 2023. It is now read-only.

Refactor search builder #21

Closed
wants to merge 2 commits into from
Closed

Conversation

Tijawk
Copy link

@Tijawk Tijawk commented Mar 26, 2023

  • upgrade dependencies and add MiniSearch as search engine
  • remove options.ts. Options are merge in index.ts directly.
  • remove baseSrc, docsPath and partialsToIgnore and add translations options.
  • refactor searchBuilder to use VITEPRESS_CONFIG.pages
  • rename preambuleTransformer to dataTransformer
  • rename FileSearchData type to IndexedData
  • add DataTransformer and Options type files
  • move search logic to useSimpleSearch to allow create custom Search.vue component
  • move styles in assets/simpleSearch.css
  • remove vue-tsc and add vite-plugin-dts to build .d.ts files
  • update README.md
  • add unit tests and github CI

New options

export type Options = {
  /**
   * Absolute path to search component.
   *
   * @default 'vitepress-plugin-simple-search/SimpleSearch.vue'
   * @type {string}
   * @memberof Options
   * @example 'absolute/path/to/MyCustomSearch.vue'
   */
  searchComponentPath?: string;

  /**
   * Search options to customize the search behavior.
   *
   * @type {SearchOptions}
   * @memberof Options
   */
  searchOptions?: SearchOptions;

  /**
   * Used to transform the generated data for each page.
   * This is useful if you want to add additional data to the search index.
   * By default search is scanning 'title' and 'text' field only.
   * You can add additional fields to the search index by using searchOptions.
   *
   * @type {(data: IndexedData) => IndexedData | Promise<IndexedData> | undefined}
   * @memberof Options
   */
  transform?: (data: IndexedData) => IndexedData | Promise<IndexedData>;
  
  /**
   * Maximum number of characters to display in the preview.
   * 
   * @default 64
   * @type {number}
   * @memberof Options
   */
  previewLength?: number;

  /**
   * Loading text on search component.
   *
   * @default 'Loading...'
   * @type string
   * @memberof Options
   */
  loadingText?: string;

  /**
   * Search text on navbar.
   *
   * @default 'Search...'
   * @type string
   * @memberof Options
   */
  searchText?: string;

  /**
   * Placeholder text on search input.
   *
   * @default 'Search. Use double quotes for stricter results.'
   * @type string
   * @memberof Options
   */
  placeholderText?: string;

  /**
   * Text to display when no results are found.
   *
   * @default 'No results found.'
   * @type string
   * @memberof Options
   */
  noResultsText?: string;

  /**
   * Text to display when search is complete.
   *
   * @default '{time}ms for {count} results'
   * @type string
   * @memberof Options
   */
  searchTimeText?: string;
};

@Stuyk
Copy link
Owner

Stuyk commented Mar 28, 2023

Is this ready for review now?

@Tijawk Tijawk marked this pull request as draft March 29, 2023 17:20
@Tijawk
Copy link
Author

Tijawk commented Mar 29, 2023

@Stuyk I will add some unit tests and I'm looking on minisearch package for the search engine. Do you agree to let me use this package instead of custom search engine ?

https://www.npmjs.com/package/minisearch

@Stuyk
Copy link
Owner

Stuyk commented Mar 30, 2023

@Tijawk I think it's fine to replace the search; the implementation I put in was enough for my needs. :)

@Tijawk
Copy link
Author

Tijawk commented Apr 2, 2023

@Stuyk You can take a quick look on my PR.

If it's ok, I will rebase commits to have a more readable git history and make it ready to final review

@Stuyk Stuyk self-requested a review April 3, 2023 03:22
Copy link
Owner

@Stuyk Stuyk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had quite a few issues on my side; but after a little bit of work I did manage to get it working. Here's what I had to change to make it compatible.

I have no idea why but the Map type was just giving out tons of errors / issues.

Replacing with an Object seemed to have resolved all of it.

src/index.ts Show resolved Hide resolved
src/dataBuilder.ts Outdated Show resolved Hide resolved
src/composables/useSimpleSearch.ts Outdated Show resolved Hide resolved
vite-env.d.ts Outdated Show resolved Hide resolved
src/composables/useSimpleSearch.ts Show resolved Hide resolved
src/__tests__/searchBuilder.test.ts Outdated Show resolved Hide resolved
src/__tests__/searchBuilder.test.ts Outdated Show resolved Hide resolved
src/__tests__/useSimpleSearch.test.ts Outdated Show resolved Hide resolved
src/index.ts Outdated Show resolved Hide resolved
src/index.ts Show resolved Hide resolved
- upgrade dependencies
- update plugin options.
- update types.
- refactor builder to use VITEPRESS_CONFIG.pages
- move search logic to useSimpleSearch to allow create custom Search.vue component
- move styles in assets/simpleSearch.css
- remove vue-tsc and add vite-plugin-dts to build .d.ts files
- update README.md
- add unit tests and github ci
@Stuyk
Copy link
Owner

Stuyk commented Apr 8, 2023

Works pretty well overall; but I only have a handful of issues with the changes.

  1. Variables are displayed from front matter and are not parsed.

Before:

After:

  1. Useless content is being appended to the search results.
  2. Initial search data, or random pages are not show outright when opening the search.

@Stuyk
Copy link
Owner

Stuyk commented Apr 11, 2023

I'm not sure we need pursue this offline search functionality any further after Vitepress finally added offline search with minisearch.

You can see the merged PR here: vuejs/vitepress#2110

This repository is likely going to get archived after this.

@Stuyk Stuyk closed this Apr 11, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants