Search across docs, GitHub issue and discussion. #859

yujonglee · 2024-10-14T10:52:16Z

Important

Adds CanarySearch component for searching docs and GitHub issues, with styling and dependency updates.

New Feature:
- Adds CanarySearch component in CanarySearch.tsx for searching across docs and GitHub issues/discussions.
- Integrates CanarySearch into the search configuration in theme.config.tsx.
Dependencies:
- Adds @getcanary/web to dependencies in package.json.
Styling:
- Adds custom styles for Canary components in style.css.

^{This description was created by}^{for 01b4c96. It will automatically update as commits are pushed.}

vercel · 2024-10-14T10:52:21Z

@yujonglee is attempting to deploy a commit to the langfuse Team on Vercel.

A member of the Team first needs to authorize it.

CLAassistant · 2024-10-14T10:52:22Z

All committers have signed the CLA.

ellipsis-dev

❌ Changes requested. Reviewed everything up to 01b4c96 in 36 seconds

More details

Looked at 136 lines of code in 4 files
Skipped 2 files when reviewing.
Skipped posting 2 drafted comments based on config settings.

1. components/CanarySearch.tsx:6

Draft comment:
Avoid using JSON.stringify for the TABS constant. Pass it as an object directly to the tabs prop.
Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable:
The comment is suggesting a code quality improvement by recommending the use of an object instead of a stringified JSON. This is a valid suggestion as it can improve readability and maintainability of the code. The use of JSON.stringify is unnecessary if the tabs prop can accept an object directly.
I might be missing the context of whether the tabs prop requires a string or an object. If it requires a string, then the use of JSON.stringify is justified.
The suggestion to use an object directly is generally a good practice unless the prop specifically requires a string. Without evidence that a string is required, the suggestion seems valid.
The comment is a valid suggestion for a code quality improvement, recommending the use of an object instead of a stringified JSON. It should be kept.

2. style.css:100

Draft comment:
Ensure that the styles for canary-trigger-searchbar are specific enough to avoid conflicts with other components.
Reason this comment was not posted:
Confidence changes required: 50%
The CSS custom properties for the canary components are defined correctly, but the canary-trigger-searchbar styles could be more specific to avoid potential conflicts with other styles.

Workflow ID: wflow_f0o8QSWZfENGl820

Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

ellipsis-dev · 2024-10-14T10:53:06Z

components/CanarySearch.tsx

+      ),
+      import("@getcanary/web/components/canary-filter-tabs-glob.js"),
+      import("@getcanary/web/components/canary-footer.js"),
+    ]).then(() => {


Consider adding error handling to the Promise.all to handle any potential import failures gracefully.

greptile-apps

Disclaimer: Experimental PR review

PR Summary

This PR introduces a new CanarySearch component, enhancing search functionality across documentation and GitHub issues/discussions, while updating related configurations and styles.

Added components/CanarySearch.tsx with dynamic imports for Canary web components
Updated package.json to include '@getcanary/web' version 1.0.9 dependency
Modified style.css with custom properties for canary-root and canary-trigger-searchbar styling
Replaced existing search in theme.config.tsx with new CanarySearch component
Changed tsconfig.json module and moduleResolution settings to 'Preserve' and 'Bundler' respectively

_{5 file(s) reviewed, no comment(s)}
_{Edit PR Review Bot Settings | Greptile}

marcklingen · 2024-10-15T06:29:57Z

components/CanarySearch.tsx

+    },
+  ]);
+
+  React.useEffect(() => {


is this necessary or could we use next dynamic imports to achieve the same result?

https://nextjs.org/docs/pages/building-your-application/optimizing/lazy-loading#nextdynamic

AFAIK, If we really want to squeeze it, than this is something we can do instead of next/dynamic: https://nextjs.org/docs/pages/building-your-application/optimizing/lazy-loading#with-external-libraries

So I guess we need slight modification in canary-input side to do that. (support initial type callback)

But Canary is already very small.

Will track it here, and bump version here once I figure it out.

I think async loading is good here, 90kb is kinda small but needs to be async/lazy in some form and not part of the core bundle.

Screen.Recording.2024-10-17.at.8.10.03.PM.mov

Did some research on this, and it seems it will be not part of the core bundle.
Since import() is placed only inside useEffect, webpack make separate bundle for each component, and load them after other part of the app(in this case, FeatureBento.tsx, Pricing.tsx, etc...)

marcklingen · 2024-10-15T06:31:30Z

components/CanarySearch.tsx

+
+  return (
+    <canary-root framework="nextra">
+      <canary-provider-cloud project-key="cpa0aae64b">


Based on the canary docs, canary is not available without canary cloud when using nextra and you added a key here. Do you know whether a local options exists as keyword search would be nice already across github and docs?

For Nextra, hooking into the build step and configuring a custom local search index is a bit more involved, but it is doable.

If you really want to avoid hosted search and leverage existing sources like https://github.com/langfuse/langfuse-docs/blob/main/src/langfuse_github_discussions.json, I can write a small script that hooks into the webpack build step to index statically generated HTML and the langfuse_github_discussions.json file. You can then search it using canary-provider-pagefind.

However, 1 - I need issue #865 to be resolved, and 2 - it might take some time.

Just FYI, Canary is self-hostable. You can start using it now and move to self-hosting later.

Feel free to clarify if you're only interested in local search at this time, which is perfectly fine.

thanks for the clarification!

Is there a way for maintainers to use/access/configure the hosted search? I cannot find public pricing or any information on canary cloud

Self-hosting this is currently a bad use of our time/effort. I'd rather keep the current search, or go for something new like canary that is either local or managed-cloud

@marcklingen

We have pricing page here: https://getcanary.dev/docs/cloud/platform/pricing
Should take <3min to setup. https://getcanary.dev/docs/cloud/platform/manual

Canary Cloud is at https://cloud.getcanary.dev

marcklingen · 2024-10-15T06:32:21Z

thanks for the contribution @yujonglee, canary looks good and could be a great replacement of the default nextra search. Can you share your thoughts on my notes above?

marcklingen · 2024-11-27T22:47:44Z

closing this for now as the default search is ok and I want to keep this self-contained and OSS for now

marcklingen · 2024-11-27T22:48:02Z

thank you for sharing this implementation!

replace search with canary

01b4c96

ellipsis-dev bot reviewed Oct 14, 2024

View reviewed changes

greptile-apps bot reviewed Oct 14, 2024

View reviewed changes

marcklingen reviewed Oct 15, 2024

View reviewed changes

marcklingen closed this Nov 27, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Search across docs, GitHub issue and discussion. #859

Search across docs, GitHub issue and discussion. #859

yujonglee commented Oct 14, 2024 •

edited by ellipsis-dev bot

Loading

vercel bot commented Oct 14, 2024

CLAassistant commented Oct 14, 2024 •

edited

Loading

ellipsis-dev bot left a comment

ellipsis-dev bot Oct 14, 2024

greptile-apps bot left a comment

marcklingen Oct 15, 2024

yujonglee Oct 15, 2024 •

edited

Loading

marcklingen Oct 15, 2024

yujonglee Oct 17, 2024

marcklingen Oct 15, 2024

yujonglee Oct 15, 2024 •

edited

Loading

marcklingen Oct 15, 2024

yujonglee Oct 17, 2024

marcklingen commented Oct 15, 2024

marcklingen commented Nov 27, 2024

marcklingen commented Nov 27, 2024

Search across docs, GitHub issue and discussion. #859

Search across docs, GitHub issue and discussion. #859

Conversation

yujonglee commented Oct 14, 2024 • edited by ellipsis-dev bot Loading

vercel bot commented Oct 14, 2024

CLAassistant commented Oct 14, 2024 • edited Loading

ellipsis-dev bot left a comment

Choose a reason for hiding this comment

ellipsis-dev bot Oct 14, 2024

Choose a reason for hiding this comment

greptile-apps bot left a comment

Choose a reason for hiding this comment

Disclaimer: Experimental PR review

PR Summary

marcklingen Oct 15, 2024

Choose a reason for hiding this comment

yujonglee Oct 15, 2024 • edited Loading

Choose a reason for hiding this comment

marcklingen Oct 15, 2024

Choose a reason for hiding this comment

yujonglee Oct 17, 2024

Choose a reason for hiding this comment

marcklingen Oct 15, 2024

Choose a reason for hiding this comment

yujonglee Oct 15, 2024 • edited Loading

Choose a reason for hiding this comment

marcklingen Oct 15, 2024

Choose a reason for hiding this comment

yujonglee Oct 17, 2024

Choose a reason for hiding this comment

marcklingen commented Oct 15, 2024

marcklingen commented Nov 27, 2024

marcklingen commented Nov 27, 2024

yujonglee commented Oct 14, 2024 •

edited by ellipsis-dev bot

Loading

CLAassistant commented Oct 14, 2024 •

edited

Loading

yujonglee Oct 15, 2024 •

edited

Loading

yujonglee Oct 15, 2024 •

edited

Loading