Repository navigation
Fix the code review findings: share links, storage, dialogs, CI - #253
Merged
Merged
Conversation
… prompt Native dialogs block the page, ignore the theme and are suppressed in sandboxed iframes, where confirm() returns false and the unsaved-changes flow silently loses work. dialogs.ts adds showAlert, showConfirm, showChoice, showCopyValue and showToast, rendered inside the Yasgui root with focus trapping, Escape to cancel and focus returned to the opener. All 41 alert/confirm/prompt calls use them; save-modal validation is shown on the field itself. Also fixes saving a Git query after a stale version tag: when the automatic retry succeeded, the code fell through to the conflict error and skipped updating the saved-version metadata.
Storage.set called the quota callback and then threw anyway, and the Yasgui callback cleared every tab, endpoint and credential. The callback now replaces the throw. Yasgui retries without the query results kept with the tabs and otherwise keeps working from memory with one message; Yasr and Yasqe only skip the write. Quota errors are recognised by name in current browsers, and the expired-key sweep (which parses all of localStorage) runs at most once a minute, with get() checking expiry itself. Also adds Yasqe requestSendsCredentials(), used by the share-link check.
A link could name any query and endpoint, and it ran at once with the credentials saved for that endpoint, so a crafted link could run an update as whoever opened it. Updates never run from a link now; queries with syntax errors, or that would send an Authorization/API key header or cookies, wait for the user to click Run query in a toast. New autoRunSharedQueries option: safe (default), always, never. Malformed headers/outputSettings JSON in a link is ignored instead of throwing in the constructor. Yasgui.dialogs exposes the dialogs to plugins and embedders. Browser tests can use another port with TEST_PORT.
… origin The postMessage handler took a callback from any window. The code verifier and state are no longer written to sessionStorage (only the closure reads them), and unexpected errors while polling the popup now reject the flow instead of being swallowed.
The import matched literals with regular expressions and paired them by position, so a tab without an endpoint shifted the endpoints of the next tabs, quotes in names broke parsing, and a tab orientation could become the global one. Adds an export/import round-trip test.
The unit-test script ended in '|| exit 0', util:validateTs (failing on yasqe tooltip.ts) and lint were never run in CI, and the lint glob pointed at a folder that no longer exists. build.yml now type-checks, lints, builds, checks bundle sizes against a budget (npm run util:bundleSize, which also lists what each bundle is made of) and runs all tests; test.yaml ran the same steps a second time and is removed. Dependabot covers npm, the website and GitHub Actions. Dependencies: query-string 10 (drops decode-uri-component, which had a high-severity advisory; npm audit is now clean). Removes packages nothing imports (choices.js, blueimp-md5, es6-object-assign in yasgui; jquery and its dead helpers, datatables.net, column-resizer, colors and free-solid-svg-icons in yasr), lists the five plugins yasgui imports as its dependencies, and moves test and build tooling to devDependencies.
The image used the default nginx server: no compression for a 5 MB bundle and no cache policy. docker/nginx.conf gzips text assets (about 1.5 MB transferred), caches the hashed fonts and images for a year, revalidates yasgui.min.js/css, and sets nosniff and a referrer policy. The build stage moves from Node 20 to 24, as in the publish workflow.
Also points the README documentation links at matgui-doc.matdata.eu, the domain the app links to, and aligns the Docusaurus url with it.
packages/yasgui/src/version.ts is generated by the build and git-ignored, so util:validateTs failed on a fresh checkout before it.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Requested by Mathias · project thread
Fixes the findings of the MatGUI code review. There is one commit per topic, so you can read it and rebase-merge it commit by commit.
Security
fix(yasgui): don't run risky share-link queries on load). Before, a link could name any query and endpoint, and it ran at once with the credentials saved for that endpoint. Now updates never run from a link. Invalid queries, and queries that would send an Authorization or API key header or cookies, wait until you click Run query in a toast. The newautoRunSharedQueriesoption takes"safe"(default),"always"or"never".npm audit --omit=dev). query-string 10 drops decode-uri-component, which had a high advisory. Unused deps are removed: choices.js (with its deepmerge advisory), blueimp-md5, es6-object-assign, jquery and its dead helpers, datatables.net, column-resizer, colors and free-solid-svg-icons. The five plugins that yasgui imports are now listed in its package.json.Correctness
Storage.setcalled the quota callback and then threw anyway, and the yasgui callback cleared every tab, endpoint and credential. Now it retries without the stored query results. If that still doesn't fit, it keeps working from memory and shows one message.headersoroutputSettingsin a link are ignored instead of leaving the page blank.UX
alert,confirmandpromptcalls. They are available asYasgui.dialogs:showAlert,showConfirm,showChoice,showCopyValueandshowToast. They follow the theme, trap focus, cancel on Escape, return focus when closed, and work in sandboxed iframes. Validation in the save modal now shows on the field itself.CI and build
unit-testno longer ends in|| exit 0. Type-check and lint now run in CI, after the build, which generatesversion.ts. Both were broken and are fixed. The duplicatetest.yamlis removed.npm run util:bundleSizelists what each bundle is made of and fails when a bundle goes over its budget. Today yasgui is about 5.1 MB minified and 1.5 MB gzipped.nginx.confadds gzip and cache headers, and the build runs on Node 24.Docs
autoRunSharedQueriesandYasgui.dialogs.urlmatches it.Not in this PR
yasgui.min.js. This needs a decision on how MatGUI is distributed, for example an ESM build with dynamic imports, so it's left for a follow-up. The bundle-size report shows where the bytes are.TabandTabSettingsModalinto smaller modules. This refactor is best done step by step.Tests
🤖 Generated with Claude Code
https://claude.ai/code/session_01ECW7opooHkpe4sh6LzSEh5