perf(store:vite config): Improvements for Dynamic Loading and Bundle Optimization #131

nitro56565 · 2025-02-28T11:07:48Z

Summary

This PR introduces changes aimed at reducing the initial bundle size and improving runtime performance by deferring heavy module imports until they are needed. The changes also streamline sample loading and enhance the Vite build configuration.

Changes

Dynamic Import of Markdown Processing Libraries:

Replaced static imports for @accordproject/concerto-core, @accordproject/template-engine, @accordproject/markdown-template, and @accordproject/markdown-transform with a new loadMarkdownProcessingLibs() function that loads these modules on-demand.
This lazy-loading approach helps reduce the initial load time by deferring the loading of heavy libraries until the rebuild function is called.

Refactored Rebuild Function:

The rebuild function is now debounced directly around an async function that leverages dynamic imports.
This change simplifies error handling and ensures that the rebuild process only loads the libraries when needed.

Sample Handling Enhancements:

Adjusted the store initialization to start with empty state values for key properties (e.g., sampleName, templateMarkdown) instead of preset values from the playground module.
Updated the loadSample method to dynamically import the playground sample if a matching sample isn’t found, improving modularity.

Vite Config Improvements:

Integrated an async plugin loader (getPlugins()) to dynamically load the vite-plugin-chunk-split plugin. This helps in code-splitting and optimizing bundle chunks.
Updated optimizeDeps and build options to include and exclude dependencies appropriately for now I have excluded typescript as that was one of the biggest chunk from @accordproject/template-engine/node_modules/typescript/lib/typescript.js which isn't required as of now in the runtime, leading to more efficient dependency management and build performance.
Consolidated configuration using mergeConfig for better maintainability between Vite and Vitest setups.

Screenshots or Video

GTmetrix-report-playground.accordproject.org-20250226T050752-dxnPRm2W.pdf
GTmetrix-report-regal-nougat-d1db8f.netlify.app-20250226T044504-DVcLGT7h.pdf

Related Issues

Author Checklist

Ensure you provide a DCO sign-off for your commits using the --signoff option of git commit.
Vital features and changes captured in unit and/or integration tests
Commits messages follow AP format
Extend the documentation, if necessary
Merging to main from fork:branchname

netlify · 2025-02-28T11:08:07Z

✅ Deploy Preview for ap-template-playground ready!

Name	Link
🔨 Latest commit	`53e009a`
🔍 Latest deploy log	https://app.netlify.com/projects/ap-template-playground/deploys/68b2957780cd2d000801b5bd
😎 Deploy Preview	https://deploy-preview-131--ap-template-playground.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

Vinyl-Davyl · 2025-02-28T15:27:54Z

Thanks @nitro56565 Looks great to me! The Asynchronous plugin loader was of great help here, Noticed the modifications you made to the store initialization process and use.

Could you please explain the use case of the new library added in the package.json vite-plugin-chunk-split?

nitro56565 · 2025-02-28T16:12:37Z

Hey @Vinyl-Davyl Thank you for going through my code,
Sure I I'll explain the use case of this plugin, vite-plugin-chunk-split is used to automates and enhances the process of code splitting, making our production builds leaner and more performant without requiring manual configuration for every dependency. This is especially beneficial for a complex application like ours where managing bundle sizes manually can be challenging. I have attached a screenshot showcasing the difference in the build which we are getting currently this diff is helping the application to load faster as we have separate minor chunks than before.

sanketshevkar · 2025-03-05T05:16:44Z

Thanks @nitro56565 this PR looks quite extensive and helpful. Can please help with some clarity on the following:

This change simplifies error handling and ensures that the rebuild process only loads the libraries when needed.

Do you have any example where error handling was improved because of this?

Adjusted the store initialization to start with empty state values for key properties (e.g., sampleName, templateMarkdown) instead of preset values from the playground module.

What does this optimise?

Updated the loadSample method to dynamically import the playground sample if a matching sample isn’t found, improving modularity.

I don't think we take input to load any sample that the app may want to match to. All samples are hard coded as of for now. How does this improve modularity?

Updated optimizeDeps and build options to include and exclude dependencies appropriately for now I have excluded typescript as that was one of the biggest chunk from @accordproject/template-engine/node_modules/typescript/lib/typescript.js which isn't required as of now in the runtime, leading to more efficient dependency management and build performance.

Afaik know template-mark in template-engine is a mix of markdown and typescript. To validate the typescript code we have written in box of template-mark in playground we need typescript as a dependency at run time instead having it as a dev deps.

sanketshevkar · 2025-03-05T05:20:05Z

vite.config.ts

  optimizeDeps: {
-    include: ["immer"],
+    include: ["immer", "zustand"],
+    exclude: ["typescript"],


I'm not sure if this excludes typescript package from template-engine to get bundled with playground. Also we do need this a deps rather than a dev-deps. I checked the PR deployed code.

The error we see on the top is thrown from the typescript compiler in template-engine.

sanketshevkar · 2025-03-05T05:20:54Z

vite.config.ts

  },
+  build: {
+    rollupOptions: {
+      external: ["typescript"],


Or even this for that matter.

nitro56565 · 2025-03-05T07:13:10Z

Hi @sanketshevkar Thank you for taking out time to review my PR.

Do you have any example where error handling was improved because of this?

by dynamically importing the rebuild processing libraries, we now have a single point of failure (loadMarkdownProcessingLibs()), which makes it easier to add centralized error handling even though I haven't added a try catch block around the loadMarkdownProcessingLibs() in this change, I can work around implementing that. but as of previous implementation static imports meant that any issue with these libraries could cause failures throughout the entire app lifecycle. which wont happen with the current code as the structure of the code now allows for better error isolation.

I don't think we take input to load any sample that the app may want to match to. All samples are hard coded as of for now. How does this improve modularity?

These changes are reallly about future-proofing the code. By starting with empty state values, the store isn’t tied to template preset defaults, making it easier to adjust and avoid assumptions about initial template.
Similarly, switching to dynamic imports for sample loading isn’t just for modularity now. the changes are made to supports shared template loading ensuring the shared template is loaded and also the way for future features, to handle situation like converting file uploads into Accord templates using AI as it may become a part of our frontend this approach would be much more feasible for the same reason.

Afaik know template-mark in template-engine is a mix of markdown and typescript. To validate the typescript code we have written in box of template-mark in playground we need typescript as a dependency at run time instead having it as a dev deps.
The error we see on the top is thrown from the typescript compiler in template-engine.

I did check for the code and also tested it, and yes you are correct about it I looked for alternatives but haven’t found any viable ones yet, therefore I'll put the typescript back in the bundle (this would degrade the performance of the whole application but we need this at runtime rightnow to process the markdown), I also check if we could dynamically load the typescript from Frontend instead of package and use that for the markdown but I dont think that would technically work.
I'll make the change for this as well and fix the conflict which I see in package-lock.
Is there anything else @sanketshevkar which you may have noticed?

Signed-off-by: nitro56565 <[email protected]>

Owaiseimdad · 2025-03-10T03:07:08Z

src/store/store.ts

+async function loadMarkdownProcessingLibs() {
+  try {
+    const [{ ModelManager }, { TemplateMarkInterpreter }, { TemplateMarkTransformer }, { transform }] =
+      await Promise.all([


@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?

@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

Owaiseimdad · 2025-03-10T03:09:24Z

src/store/store.ts

+  try {
+    const [{ ModelManager }, { TemplateMarkInterpreter }, { TemplateMarkTransformer }, { transform }] =
+      await Promise.all([
+        import("@accordproject/concerto-core"),


This is not scalable, use a array and create a map of it inside the promise.
Ex:
`
var mod = [
"@accordproject/concerto-core",
"@accordproject/template-engine",
... more
]

await Promise.all(mod.map(module => import(module)))
`

Something like this.

@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?

Owaiseimdad · 2025-03-10T03:11:53Z

src/store/store.ts

-      editorModelCto: playground.MODEL,
-      data: JSON.stringify(playground.DATA, null, 2),
-      editorAgreementData: JSON.stringify(playground.DATA, null, 2),
+      sampleName: "",


Use of playground was to avoid using the empty string in first place. Any reason why playground is removed.

@Owaiseimdad the initial load is kept empty considering future plans of pulling up the record from local storage and keeping it persistent even after refresh , also this approach will also help us while implementing handling AI tuned data (future plan to implement upload file and populate system).

Owaiseimdad · 2025-03-10T03:14:58Z

src/store/store.ts

+    console.error("Error rebuilding the template:", error);
+    throw new Error("Failed to process the template");
+  }
+}, 500);


Why 500ms and not 300ms or more than 500ms?

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.

Owaiseimdad · 2025-03-10T03:15:36Z

src/store/store.ts

+  }
 }

+const rebuild = debounce(async (template: string, model: string, dataString: string) => {


Why not use throttle here? Any particular reason for debounce?

Maybe we can experiment with throttling, but let's keep it for a separate issue.

@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.

nitro56565 · 2025-03-15T10:16:16Z

Hi @sanketshevkar I have made some changes and updated this PR, can you go though it?

sanketshevkar · 2025-03-15T10:34:33Z

by dynamically importing the rebuild processing libraries, we now have a single point of failure (loadMarkdownProcessingLibs()), which makes it easier to add centralized error handling even though I haven't added a try catch block around the loadMarkdownProcessingLibs() in this change, I can work around implementing that. but as of previous implementation static imports meant that any issue with these libraries could cause failures throughout the entire app lifecycle. which wont happen with the current code as the structure of the code now allows for better error isolation.

I'm still confused, you've mentioned centralised error handling in the start and in the end you've mentioned that the changes allow better error isolation. Is there any example where I can see this, where an actual error is thrown?

static imports meant that any issue with these libraries could cause failures throughout the entire app lifecycle

Do you have an example of this and how your changes fixes that failure? What kind of failures are we talking about?

sanketshevkar · 2025-03-15T10:38:37Z

this would degrade the performance of the whole application but we need this at runtime rightnow to process the markdown

Is there any numerical evidence for this slowdown? From what I understand, even with your changes typescript from template-engine was getting bundled with the app and that is what we require. And even without your changes typescript will get bundled. Where is the slowdown here?

sanketshevkar · 2025-03-15T10:42:35Z

@nitro56565 can you also please address @Owaiseimdad 's comments?

nitro56565 · 2025-03-15T13:17:51Z

Hi @sanketshevkar, Let me clarify the points around centralized error handling and error isolation:
When I mentioned centralized error handling, I meant that with loadMarkdownProcessingLibs() acting as the single entry point for loading these libraries, it’s now easier to wrap the dynamic imports in one try-catch block.

Error Isolation was not in term of separating the errors but to handle failures as deferred until runtime. If any one of the libraries failed to load example a corrupted module, missing dependency, or a breaking library update, the entire app would fail on startup, leading to a white screen, allowing the app to not stay functional.

Currently I dont have an example to demonstrate an actual package failure but I can simulate an example

without dynamic imports and no error handling situation

with dynamic imports and error handling situation

nitro56565 · 2025-03-15T13:29:05Z

Is there any numerical evidence for this slowdown? From what I understand, even with your changes typescript from template-engine was getting bundled with the app and that is what we require. And even without your changes typescript will get bundled. Where is the slowdown here?

@sanketshevkar When I had excluded the typescript which was coming from template engine it was not actually getting bundled up with with our UI bundle and typescript is a pretty heavy package, I am attaching some before and after visuals of what exactly the performance degradation happend.

With Typescript from Template Engine

Without Typescript from Engine

nitro56565 · 2025-03-15T13:36:38Z

I'm not sure if this excludes typescript package from template-engine to get bundled with playground. Also we do need this a deps rather than a dev-deps. I checked the PR deployed code.

Ohh I guess there is some confusion here, I was trying to remove the typescript before this PR at that time I was trying to manipulate the template engine from that repo by pushing the typescript as dev deps but when you told me about the need of typescript(for logic) from the above message and through discord communication I stopped working on that part and I only excluded the typescript from playground bundle and not template engine bundle.

note - Currently as you mentioned that we do need typescript I have put back the typescript package back to playground bundle.

nitro56565 requested a review from a team as a code owner February 28, 2025 11:07

nitro56565 changed the title ~~dynamic loading and bundle optimization~~ perf(store:vite config): Improvements for Dynamic Loading and Bundle Optimization Feb 28, 2025

nitro56565 force-pushed the nitro56565/code-optimization-and-refactor branch from ef41d90 to d01837e Compare February 28, 2025 12:41

sanketshevkar reviewed Mar 5, 2025

View reviewed changes

vite.config.ts Outdated

},

build: {

rollupOptions: {

external: ["typescript"],

Copy link

Member

sanketshevkar Mar 5, 2025

Choose a reason for hiding this comment

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

Or even this for that matter.

nitro56565 added 3 commits March 5, 2025 13:14

dynamic loading and bundle optimization

e83c87b

Signed-off-by: nitro56565 <[email protected]>

moved build to viteconfig

977d98d

Signed-off-by: nitro56565 <[email protected]>

fixed error handling in store and reinitiated typescript

9387446

Signed-off-by: nitro56565 <[email protected]>

nitro56565 force-pushed the nitro56565/code-optimization-and-refactor branch from d01837e to 9387446 Compare March 5, 2025 07:46

nitro56565 requested a review from sanketshevkar March 6, 2025 05:58

Owaiseimdad reviewed Mar 10, 2025

View reviewed changes

Merge branch 'main' into nitro56565/code-optimization-and-refactor

53e009a

Uh oh!

perf(store:vite config): Improvements for Dynamic Loading and Bundle Optimization #131

Are you sure you want to change the base?

perf(store:vite config): Improvements for Dynamic Loading and Bundle Optimization #131

Uh oh!

Conversation

nitro56565 commented Feb 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Changes

Dynamic Import of Markdown Processing Libraries:

Refactored Rebuild Function:

Sample Handling Enhancements:

Vite Config Improvements:

Screenshots or Video

Related Issues

Author Checklist

Uh oh!

netlify bot commented Feb 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for ap-template-playground ready!

Uh oh!

Vinyl-Davyl commented Feb 28, 2025

Uh oh!

nitro56565 commented Feb 28, 2025

Uh oh!

sanketshevkar commented Mar 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nitro56565 commented Mar 5, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nitro56565 commented Mar 15, 2025

Uh oh!

sanketshevkar commented Mar 15, 2025

Uh oh!

sanketshevkar commented Mar 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sanketshevkar commented Mar 15, 2025

Uh oh!

nitro56565 commented Mar 15, 2025

Uh oh!

nitro56565 commented Mar 15, 2025

Uh oh!

nitro56565 commented Mar 15, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

nitro56565 commented Feb 28, 2025 •

edited

Loading

netlify bot commented Feb 28, 2025 •

edited

Loading

sanketshevkar commented Mar 5, 2025 •

edited

Loading

sanketshevkar commented Mar 15, 2025 •

edited

Loading