Skip to content

feat: support global search in multiple environments and fix pipeline errors #2629

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Dec 9, 2024
Merged

feat: support global search in multiple environments and fix pipeline errors #2629

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5e7c317
feat: support global search in multiple environments and fix pipeline…
zzcr Dec 9, 2024
770a226
feat: support global search in multiple environments and fix pipeline…
zzcr Dec 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions examples/sites/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@opentiny/vue-docs",
"version": "3.20.3",
"version": "3.20.5",
"license": "MIT",
"scripts": {
"start": "vite",
Expand Down Expand Up @@ -49,7 +49,8 @@
"vue-i18n": "^9.1.10",
"vue-router": "4.1.5",
"@docsearch/js": "^3.8.0",
"@docsearch/css": "^3.8.0"
"@docsearch/css": "^3.8.0",
"@docsearch/react": "npm:@docsearch/css"
Comment on lines +52 to +53
Copy link

Choose a reason for hiding this comment

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

💡 Codebase verification

🛠️ Refactor suggestion

The @docsearch/react dependency configuration is incorrect and should be fixed

The codebase is using @docsearch/js and @docsearch/css directly, as evidenced in main.js:

import docsearch from '@docsearch/js'
import '@docsearch/css'

The @docsearch/react dependency is incorrectly aliased to @docsearch/css and should be removed since:

  1. The project uses the vanilla JS version (@docsearch/js), not the React version
  2. CSS is already properly imported via the direct @docsearch/css dependency
     "@docsearch/js": "^3.8.0",
     "@docsearch/css": "^3.8.0",
-    "@docsearch/react": "npm:@docsearch/css"
🔗 Analysis chain

Verify @docsearch/react dependency configuration

The dependency configuration appears unusual:

  1. @docsearch/react is aliased to @docsearch/css
  2. @docsearch/css is also listed as a direct dependency

This might lead to duplicate CSS imports and potential version conflicts.

Consider using the proper @docsearch/react package instead of the CSS alias:

     "@docsearch/js": "^3.8.0",
     "@docsearch/css": "^3.8.0",
-    "@docsearch/react": "npm:@docsearch/css"
+    "@docsearch/react": "^3.8.0"

The previous commands didn't provide enough context. Let's search more broadly for docsearch usage and check package configurations to understand the dependency setup better.

Let's examine how docsearch is actually being used in the project to better understand the dependency configuration.

Let's examine how docsearch is actually implemented in the main.js file and if there are any other relevant files to understand the dependency setup better.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check for potential duplicate CSS imports
rg -l "from ['|\"]@docsearch/(css|react)['|\"]" 

# Verify package resolution
npm ls @docsearch/css @docsearch/react

Length of output: 146

Script:

#!/bin/bash
# Let's check for any imports or usage patterns of @docsearch in the codebase
rg -l "@docsearch" --type js --type ts --type vue

# Check if there are any other package.json files that might be using @docsearch
fd package.json --type f --exec grep -l "@docsearch" {}

# Look for any CSS imports related to docsearch
rg -l "docsearch" --type css

Length of output: 223

Script:

#!/bin/bash
# Let's examine the content of style.css to understand how docsearch is being used
cat examples/sites/src/style.css

# Check the exact content around docsearch in package.json for more context
grep -A 5 -B 5 "@docsearch" examples/sites/package.json

# Look for any JavaScript/TypeScript files that might be using docsearch
rg -l "docsearch" --type js --type ts

Length of output: 1830

Script:

#!/bin/bash
# Let's check the main.js file to see how docsearch is being used
cat examples/sites/src/main.js

# Check if there are any other JS files that might contain docsearch configuration
rg -l "docsearch" --type js --type ts -g '!main.js'

# Look for any Vue components that might be using docsearch
fd -e vue -x grep -l "docsearch" {}

Length of output: 2682

},
"devDependencies": {
"@opentiny-internal/unplugin-virtual-template": "workspace:~",
Expand Down
20 changes: 12 additions & 8 deletions examples/sites/src/main.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,21 +33,25 @@ import css from 'highlight.js/lib/languages/css'
import html from 'highlight.js/lib/languages/xml'
import docsearch from '@docsearch/js'
import '@docsearch/css'
import { doSearchEverySite } from './tools/docsearch'

const envTarget = import.meta.env.VITE_BUILD_TARGET || 'open'

hljs.registerLanguage('javascript', javascript)
hljs.registerLanguage('css', css)
hljs.registerLanguage('html', html)

if (envTarget === 'open') {
docsearch({
appId: 'AGPA5UXHMH',
apiKey: '5fa09fc20270efa61d68e2c2eb0f56df',
indexName: 'opentiny',
container: '.search-box',
debug: false
})
docsearch({
appId: 'AGPA5UXHMH',
apiKey: '5fa09fc20270efa61d68e2c2eb0f56df',
indexName: 'opentiny',
container: '.search-box',
debug: false
})
Comment on lines +44 to +50
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Critical: Move API credentials to environment variables

The Algolia API credentials are exposed in client-side code, which poses a security risk. These should be moved to environment variables.

 docsearch({
-  appId: 'AGPA5UXHMH',
-  apiKey: '5fa09fc20270efa61d68e2c2eb0f56df',
+  appId: import.meta.env.VITE_ALGOLIA_APP_ID,
+  apiKey: import.meta.env.VITE_ALGOLIA_API_KEY,
   indexName: 'opentiny',
   container: '.search-box',
   debug: false
 })
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
docsearch({
appId: 'AGPA5UXHMH',
apiKey: '5fa09fc20270efa61d68e2c2eb0f56df',
indexName: 'opentiny',
container: '.search-box',
debug: false
})
docsearch({
appId: import.meta.env.VITE_ALGOLIA_APP_ID,
apiKey: import.meta.env.VITE_ALGOLIA_API_KEY,
indexName: 'opentiny',
container: '.search-box',
debug: false
})
🧰 Tools
🪛 Gitleaks (8.21.2)

46-46: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


if (envTarget !== 'open') {
// 支持本地开发和内网使用全局搜索
doSearchEverySite()
}

// 实验后发现，先调用一次预热一下，后续再调用会有速度的提示，因此在main中预热一下。
Expand Down
26 changes: 26 additions & 0 deletions examples/sites/src/tools/docsearch.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
const HIT_CLASS = 'DocSearch-Hit'

const findUrlLink = (target) => {
if (target?.nodeName?.toLocaleLowerCase?.() === 'a') {
return target.getAttribute('href')
} else if (target?.parentElement) {
return findUrlLink(target.parentElement)
}
}
Comment on lines +3 to +9
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add error handling for recursive DOM traversal

The findUrlLink function could potentially cause a stack overflow with deeply nested DOM structures. Consider adding a maximum depth limit.

-const findUrlLink = (target) => {
+const findUrlLink = (target, depth = 0) => {
+  const MAX_DEPTH = 10;
+  if (depth > MAX_DEPTH) return null;
   if (target?.nodeName?.toLocaleLowerCase?.() === 'a') {
     return target.getAttribute('href')
   } else if (target?.parentElement) {
-    return findUrlLink(target.parentElement)
+    return findUrlLink(target.parentElement, depth + 1)
   }
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const findUrlLink = (target) => {
if (target?.nodeName?.toLocaleLowerCase?.() === 'a') {
return target.getAttribute('href')
} else if (target?.parentElement) {
return findUrlLink(target.parentElement)
}
}
const findUrlLink = (target, depth = 0) => {
const MAX_DEPTH = 10;
if (depth > MAX_DEPTH) return null;
if (target?.nodeName?.toLocaleLowerCase?.() === 'a') {
return target.getAttribute('href')
} else if (target?.parentElement) {
return findUrlLink(target.parentElement, depth + 1)
}
}


const isAlgoliaHitDom = (dom) =>
dom?.className?.includes?.(HIT_CLASS) || dom?.parentElement?.className?.includes?.(HIT_CLASS)

export const doSearchEverySite = () => {
window.addEventListener('click', (event) => {
const target = event.target
if (isAlgoliaHitDom(target)) {
const openUrl = findUrlLink(target)
if (openUrl) {
const urlObj = new URL(openUrl)
event.preventDefault()
window.location.href = openUrl.replace(urlObj.origin, window.location.origin)
}
}
})
}
Loading