Skip to content

Render Components Only If Available#57

Merged
hari-kuriakose merged 10 commits into
mainfrom
feature/pluggable-components-in-fe
Mar 5, 2024
Merged

Render Components Only If Available#57
hari-kuriakose merged 10 commits into
mainfrom
feature/pluggable-components-in-fe

Conversation

@tahierhussain

@tahierhussain tahierhussain commented Mar 4, 2024

Copy link
Copy Markdown
Contributor

What

Some of the components may or may not be available in the FE repo. For such cases, we need to gracefully handle it in the UI if the components are not available.
...

Why

We need to handle the UI if the components are not available.
...

How

Using try-catch to import the component or ignore (if not available).
...

Relevant Docs

NA

Related Issues or PRs

https://github.com/Zipstack/unstract-cloud/pull/5

Dependencies Versions / Env Variables

NA

Notes on Testing

Tested it by building the FE with and without the plugins folder. Below are the screenshots for both the cases (Please check the URLs as a proof of the builds),

1. FE without the plugins folder:
Screenshot from 2024-03-04 17-54-09

2. FE with the plugins folder:
Screenshot from 2024-03-04 18-45-42

...

Screenshots

Please refer Notes on Testing for screenshots.

...

Checklist

I have read and understood the Contribution Guidelines.

@hari-kuriakose hari-kuriakose left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@tahierhussain LGTM overall.

However, I still see references to plugin functionalities in FE code. Take eval metrics for example - sortEvalMetricsByType function, evalMetrics variable, etc are still present outside of plugins.

Can we move all such references across all plugins to the plugins folder itself?

@hari-kuriakose

Copy link
Copy Markdown
Contributor

@tahierhussain Can you also please confirm if the build process succeeded with and without plugins, under Notes on Testing?

@tahierhussain

Copy link
Copy Markdown
Contributor Author

@tahierhussain Can you also please confirm if the build process succeeded with and without plugins, under Notes on Testing?

Done, @hari-kuriakose

@tahierhussain

Copy link
Copy Markdown
Contributor Author

@tahierhussain LGTM overall.

However, I still see references to plugin functionalities in FE code. Take eval metrics for example - sortEvalMetricsByType function, evalMetrics variable, etc are still present outside of plugins.

Can we move all such references across all plugins to the plugins folder itself?

@hari-kuriakose I have made further changes and moved the logic/functions to the plugins folder.

However, in some cases, it won't be easy to avoid references to eval*. For example, we retrieve the eval results from the prompt run API, which is called inside PromptCard.jsx (a regular component). So, the evalMetrics variable will be referenced, and it will be difficult to avoid. It will still be possible if we smartly handle this case, but after I discussed this challenge with @arun-venkataswamy, he told me that having references to it in the code won't be a problem.

@hari-kuriakose

hari-kuriakose commented Mar 5, 2024

Copy link
Copy Markdown
Contributor

@tahierhussain LGTM overall.
However, I still see references to plugin functionalities in FE code. Take eval metrics for example - sortEvalMetricsByType function, evalMetrics variable, etc are still present outside of plugins.
Can we move all such references across all plugins to the plugins folder itself?

@hari-kuriakose I have made further changes and moved the logic/functions to the plugins folder.

However, in some cases, it won't be easy to avoid references to eval*. For example, we retrieve the eval results from the prompt run API, which is called inside PromptCard.jsx (a regular component). So, the evalMetrics variable will be referenced, and it will be difficult to avoid. It will still be possible if we smartly handle this case, but after I discussed this challenge with @arun-venkataswamy, he told me that having references to it in the code won't be a problem.

@tahierhussain Yes, the aim is to move all "defnition" references inside the plugin, leaving only "usage" references at max in the OSS code. This seems to be mostly achieved now, which is good enough.

@hari-kuriakose hari-kuriakose left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@tahierhussain LGTM overall.

@hari-kuriakose
hari-kuriakose merged commit 80f4399 into main Mar 5, 2024
@hari-kuriakose
hari-kuriakose deleted the feature/pluggable-components-in-fe branch March 5, 2024 07:45
mohamed-siddhiq pushed a commit that referenced this pull request Mar 7, 2024
* Render component only when available

* Handle the usage of plugin components in the FE

* Updated .gitignore

* Minor fix

* Moved the eval related functions to the plugins folder

---------

Co-authored-by: Hari John Kuriakose <hari@zipstack.com>
pk-zipstack pushed a commit that referenced this pull request Aug 20, 2025
* Render component only when available

* Handle the usage of plugin components in the FE

* Updated .gitignore

* Minor fix

* Moved the eval related functions to the plugins folder

---------

Co-authored-by: Hari John Kuriakose <hari@zipstack.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants