-
-
Notifications
You must be signed in to change notification settings - Fork 101
perf(store:vite config): Improvements for Dynamic Loading and Bundle Optimization #131
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
base: main
Are you sure you want to change the base?
Changes from all commits
e83c87b
977d98d
9387446
53e009a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,12 +2,7 @@ import { create } from "zustand"; | |
| import { devtools } from "zustand/middleware"; | ||
| import { immer } from "zustand/middleware/immer"; | ||
| import { debounce } from "ts-debounce"; | ||
| import { ModelManager } from "@accordproject/concerto-core"; | ||
| import { TemplateMarkInterpreter } from "@accordproject/template-engine"; | ||
| import { TemplateMarkTransformer } from "@accordproject/markdown-template"; | ||
| import { transform } from "@accordproject/markdown-transform"; | ||
| import { SAMPLES, Sample } from "../samples"; | ||
| import * as playground from "../samples/playground"; | ||
| import { compress, decompress } from "../utils/compression/compression"; | ||
| import { AIConfig, ChatState } from '../types/components/AIAssistant.types'; | ||
|
|
||
|
|
@@ -64,31 +59,58 @@ export interface DecompressedData { | |
| agreementHtml: string; | ||
| } | ||
|
|
||
| const rebuildDeBounce = debounce(rebuild, 500); | ||
| async function loadMarkdownProcessingLibs() { | ||
| try { | ||
| const [{ ModelManager }, { TemplateMarkInterpreter }, { TemplateMarkTransformer }, { transform }] = | ||
| await Promise.all([ | ||
| import("@accordproject/concerto-core"), | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is not scalable, use a array and create a map of it inside the promise. await Promise.all(mod.map(module => import(module))) Something like this.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @sanketshevkar do we have any plans of scaling up on core packages, I am not sure if we need to implement it by mapping each package? |
||
| import("@accordproject/template-engine"), | ||
| import("@accordproject/markdown-template"), | ||
| import("@accordproject/markdown-transform"), | ||
| ]); | ||
|
|
||
| async function rebuild(template: string, model: string, dataString: string) { | ||
| const modelManager = new ModelManager({ strict: true }); | ||
| modelManager.addCTOModel(model, undefined, true); | ||
| await modelManager.updateExternalModels(); | ||
| const engine = new TemplateMarkInterpreter(modelManager, {}); | ||
| const templateMarkTransformer = new TemplateMarkTransformer(); | ||
| const templateMarkDom = templateMarkTransformer.fromMarkdownTemplate( | ||
| { content: template }, | ||
| modelManager, | ||
| "contract", | ||
| { verbose: false } | ||
| ); | ||
| const data = JSON.parse(dataString); | ||
| const ciceroMark = await engine.generate(templateMarkDom, data); | ||
| return await transform( | ||
| ciceroMark.toJSON(), | ||
| "ciceromark_parsed", | ||
| ["html"], | ||
| {}, | ||
| { verbose: false } | ||
| ); | ||
| return { ModelManager, TemplateMarkInterpreter, TemplateMarkTransformer, transform }; | ||
| } catch (error) { | ||
| console.error("Failed to load Accord processing libraries:", error); | ||
| throw new Error("Error loading Accord processing libraries"); | ||
| } | ||
| } | ||
|
|
||
| const rebuild = debounce(async (template: string, model: string, dataString: string) => { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why not use throttle here? Any particular reason for debounce?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe we can experiment with throttling, but let's keep it for a separate issue.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @sanketshevkar @Owaiseimdad I believe debounce suites best over here, reason behind it is that user may type fast and if we implement throttling then after every interval it will update the UI instead of waiting even if the user is still typing, incase of debouncing the interval will start after the user has stopped typing therefore I think debouncing would be better here. |
||
| try { | ||
| const { ModelManager, TemplateMarkInterpreter, TemplateMarkTransformer, transform } = | ||
| await loadMarkdownProcessingLibs(); | ||
|
|
||
| const modelManager = new ModelManager({ strict: true }); | ||
| modelManager.addCTOModel(model, undefined, true); | ||
| await modelManager.updateExternalModels(); | ||
|
|
||
| const engine = new TemplateMarkInterpreter(modelManager, {}); | ||
| const templateMarkTransformer = new TemplateMarkTransformer(); | ||
|
|
||
| const templateMarkDom = templateMarkTransformer.fromMarkdownTemplate( | ||
| { content: template }, | ||
| modelManager, | ||
| "contract", | ||
| { verbose: false } | ||
| ); | ||
|
|
||
| const data = JSON.parse(dataString); | ||
|
|
||
| const ciceroMark = await engine.generate(templateMarkDom, data); | ||
| return await transform( | ||
| ciceroMark.toJSON(), | ||
| "ciceromark_parsed", | ||
| ["html"], | ||
| {}, | ||
| { verbose: false } | ||
| ); | ||
| } catch (error) { | ||
| console.error("Error rebuilding the template:", error); | ||
| throw new Error("Failed to process the template"); | ||
| } | ||
| }, 500); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why 500ms and not 300ms or more than 500ms?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we already had 500ms from before, I did'nt change the value but It would be a great suggestion checking for lower latency as well. |
||
|
|
||
| const getInitialTheme = () => { | ||
| if (typeof window !== 'undefined') { | ||
| const savedTheme = localStorage.getItem('theme'); | ||
|
|
@@ -109,14 +131,14 @@ const useAppStore = create<AppState>()( | |
| return { | ||
| backgroundColor: initialTheme.backgroundColor, | ||
| textColor: initialTheme.textColor, | ||
| sampleName: playground.NAME, | ||
| templateMarkdown: playground.TEMPLATE, | ||
| editorValue: playground.TEMPLATE, | ||
| modelCto: playground.MODEL, | ||
| editorModelCto: playground.MODEL, | ||
| data: JSON.stringify(playground.DATA, null, 2), | ||
| editorAgreementData: JSON.stringify(playground.DATA, null, 2), | ||
| agreementHtml: "", | ||
| sampleName: "", | ||
| templateMarkdown: "", | ||
| editorValue: "", | ||
| modelCto: "", | ||
| editorModelCto: "", | ||
| data: "", | ||
| editorAgreementData: "", | ||
| agreementHtml: "", | ||
| isAIConfigOpen: false, | ||
| isAIChatOpen: false, | ||
| error: undefined, | ||
|
|
@@ -152,73 +174,51 @@ const useAppStore = create<AppState>()( | |
| if (compressedData) { | ||
| await get().loadFromLink(compressedData); | ||
| } else { | ||
| await get().rebuild(); | ||
| await get().loadSample("default"); | ||
| } | ||
| }, | ||
| loadSample: async (name: string) => { | ||
| const sample = SAMPLES.find((s) => s.NAME === name); | ||
| if (sample) { | ||
| set(() => ({ | ||
| sampleName: sample.NAME, | ||
| agreementHtml: undefined, | ||
| error: undefined, | ||
| templateMarkdown: sample.TEMPLATE, | ||
| editorValue: sample.TEMPLATE, | ||
| modelCto: sample.MODEL, | ||
| editorModelCto: sample.MODEL, | ||
| data: JSON.stringify(sample.DATA, null, 2), | ||
| editorAgreementData: JSON.stringify(sample.DATA, null, 2), | ||
| })); | ||
| await get().rebuild(); | ||
| } | ||
| const playground = await import("../samples/playground"); | ||
| const sample = SAMPLES.find((s) => s.NAME === name) || playground; | ||
| set(() => ({ | ||
| sampleName: sample.NAME, | ||
| agreementHtml: undefined, | ||
| error: undefined, | ||
| templateMarkdown: sample.TEMPLATE, | ||
| editorValue: sample.TEMPLATE, | ||
| modelCto: sample.MODEL, | ||
| editorModelCto: sample.MODEL, | ||
| data: JSON.stringify(sample.DATA, null, 2), | ||
| editorAgreementData: JSON.stringify(sample.DATA, null, 2), | ||
| })); | ||
| await get().rebuild(); | ||
| }, | ||
| rebuild: async () => { | ||
| const { templateMarkdown, modelCto, data } = get(); | ||
| try { | ||
| const result = await rebuildDeBounce(templateMarkdown, modelCto, data); | ||
| set(() => ({ agreementHtml: result, error: undefined })); // Clear error on success | ||
| const result = await rebuild(templateMarkdown, modelCto, data); | ||
| set(() => ({ agreementHtml: result, error: undefined })); | ||
| } catch (error: any) { | ||
| set(() => ({ error: formatError(error), isProblemPanelVisible: true })); | ||
| } | ||
| }, | ||
| setTemplateMarkdown: async (template: string) => { | ||
| set(() => ({ templateMarkdown: template })); | ||
| const { modelCto, data } = get(); | ||
| try { | ||
| const result = await rebuildDeBounce(template, modelCto, data); | ||
| set(() => ({ agreementHtml: result, error: undefined })); // Clear error on success | ||
| } catch (error: any) { | ||
| set(() => ({ error: formatError(error), isProblemPanelVisible: true })); | ||
| } | ||
| await get().rebuild(); | ||
| }, | ||
| setEditorValue: (value: string) => { | ||
| set(() => ({ editorValue: value })); | ||
| }, | ||
| setModelCto: async (model: string) => { | ||
| set(() => ({ modelCto: model })); | ||
| const { templateMarkdown, data } = get(); | ||
| try { | ||
| const result = await rebuildDeBounce(templateMarkdown, model, data); | ||
| set(() => ({ agreementHtml: result, error: undefined })); // Clear error on success | ||
| } catch (error: any) { | ||
| set(() => ({ error: formatError(error), isProblemPanelVisible: true })); | ||
| } | ||
| await get().rebuild(); | ||
| }, | ||
| setEditorModelCto: (value: string) => { | ||
| set(() => ({ editorModelCto: value })); | ||
| }, | ||
| setData: async (data: string) => { | ||
| set(() => ({ data })); | ||
| try { | ||
| const result = await rebuildDeBounce( | ||
| get().templateMarkdown, | ||
| get().modelCto, | ||
| data | ||
| ); | ||
| set(() => ({ agreementHtml: result, error: undefined })); // Clear error on success | ||
| } catch (error: any) { | ||
| set(() => ({ error: formatError(error), isProblemPanelVisible: true })); | ||
| } | ||
| await get().rebuild(); | ||
| }, | ||
| setEditorAgreementData: (value: string) => { | ||
| set(() => ({ editorAgreementData: value })); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@nitro56565 good implementation, just that lets say even if once fails then due to promise.all, all the import should fail. Can we use promise.allSettled in this case?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Owaiseimdad If any one of the packages fail then there is no reason to hold up with other packages as they all are dependent on each other and for fallback I have put a catch block to handle the errors