Conversation
There was a problem hiding this comment.
Pull request overview
This pull request performs maintenance updates including dependency version bumps, type refinements, and implementation of a cache-busting reload mechanism.
Changes:
- Updated mathjs from 15.1.0 to 15.1.1, @biomejs/biome from 2.3.14 to 2.3.15, and @types/plotly.js from 3.0.9 to 3.0.10
- Refined type signatures in math function imports from generic
mathjs.MathTypeto explicitnumber[]andunknowntypes - Implemented cache-busting reload mechanism with
forceReloadaction to prevent error toasts during app updates
Reviewed changes
Copilot reviewed 6 out of 8 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/renderer/src/components/views/SimulationExperimentView.vue | Removed nullish coalescing operator on mathjs.all, refined type signatures for imported math functions to use explicit types instead of mathjs.MathType, added helper function for handling array/scalar values, changed evaluateValue return type to unknown with type assertion at call site |
| src/renderer/src/components/OpenCOR.vue | Added forceReload action handler with explanatory comments to suppress error toasts during update-triggered reloads |
| src/renderer/src/common/version.ts | Modified reloadApp to append forceReload action with timestamp to URL for cache-busting and error-suppression |
| src/renderer/src/common/common.ts | Bumped mathjs CDN import from version 15.1.0 to 15.1.1 |
| src/renderer/package.json | Updated application version to 0.20260213.0 and dependencies (@biomejs/biome, @types/plotly.js) |
| package.json | Updated application version to 0.20260213.0 and dependencies (@biomejs/biome, @types/plotly.js) |
| src/renderer/bun.lock | Updated lock file entries for @biomejs/biome and @types/plotly.js dependencies |
| bun.lock | Updated lock file entries for @biomejs/biome and @types/plotly.js dependencies |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 8 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 8 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (action.value.startsWith('forceReload')) { | ||
| // This action (parameter in fact) is used with a timestamp as an argument to force OpenCOR to reload | ||
| // (following an update) without getting an error toast. So, we do nothing here and we just let the | ||
| // reload to happen. | ||
| // Note: the reload is triggered by the URL change that we do in reloadApp() in version.ts. This is | ||
| // done to bypass the cache and ensure that the latest version of OpenCOR is loaded. When the | ||
| // reload happens, the URL contains the forceReload action with a timestamp argument, but since | ||
| // we are reloading anyway, we can just ignore it and not show an error toast. |
There was a problem hiding this comment.
With reloadApp() now adding ?forceReload=..., the existing if (window.location.search) { ... window.location.reload(); } logic will always trigger an immediate second reload after stripping the query string. That second reload can negate the cache-busting benefit (since it reloads the clean URL) and adds an extra page load. Consider handling forceReload in the first-phase URL parsing (before calling window.location.reload()), e.g. strip it with replaceState and continue without a reload/localStorage action.
| const value = (v: unknown, i: number): number => { | ||
| return Array.isArray(v) ? v[i] : Number(v); | ||
| }; | ||
|
|
||
| interactiveMath.import( | ||
| { | ||
| // Arithmetic operators. | ||
|
|
||
| pow: (x: mathjs.MathType, y: mathjs.MathType) => x.map((v: number) => v ** y), | ||
| sqrt: (x: mathjs.MathType) => x.map((v: number) => Math.sqrt(v)), | ||
| abs: (x: mathjs.MathType) => x.map((v: number) => Math.abs(v)), | ||
| exp: (x: mathjs.MathType) => x.map((v: number) => Math.exp(v)), | ||
| log: (x: mathjs.MathType) => x.map((v: number) => Math.log(v)), | ||
| log10: (x: mathjs.MathType) => x.map((v: number) => Math.log10(v)), | ||
| ceil: (x: mathjs.MathType) => x.map((v: number) => Math.ceil(v)), | ||
| floor: (x: mathjs.MathType) => x.map((v: number) => Math.floor(v)), | ||
| min: (x: mathjs.MathType, y: mathjs.MathType) => x.map((v: number, index: number) => Math.min(v, y[index])), | ||
| max: (x: mathjs.MathType, y: mathjs.MathType) => x.map((v: number, index: number) => Math.max(v, y[index])), | ||
| mod: (x: mathjs.MathType, y: mathjs.MathType) => x.map((v: number) => v % y), | ||
| pow: (x: number[], y: unknown) => x.map((v: number, i: number) => v ** value(y, i)), | ||
| sqrt: (x: number[]) => x.map((v: number) => Math.sqrt(v)), | ||
| abs: (x: number[]) => x.map((v: number) => Math.abs(v)), | ||
| exp: (x: number[]) => x.map((v: number) => Math.exp(v)), | ||
| log: (x: number[]) => x.map((v: number) => Math.log(v)), | ||
| log10: (x: number[]) => x.map((v: number) => Math.log10(v)), | ||
| ceil: (x: number[]) => x.map((v: number) => Math.ceil(v)), | ||
| floor: (x: number[]) => x.map((v: number) => Math.floor(v)), | ||
| min: (x: number[], y: unknown) => x.map((v: number, i: number) => Math.min(v, value(y, i))), | ||
| max: (x: number[], y: unknown) => x.map((v: number, i: number) => Math.max(v, value(y, i))), | ||
| mod: (x: number[], y: unknown) => x.map((v: number, i: number) => v % value(y, i)), |
There was a problem hiding this comment.
The new value() helper only treats plain JS arrays as element-wise inputs. In interactive plot evaluation the data series are set from locCommon.simulationData(...), which returns typed arrays (e.g. Float64Array), and those will make Array.isArray(v) false, so Number(v) yields NaN. This breaks pow/min/max/mod when the second argument is a typed array/array-like. Consider extending value() to handle typed arrays / array-like objects (e.g. ArrayBuffer.isView(v) or checking typeof v === 'object' && v != null && i in v).
1 participant
No description provided.