feat: web explorer web three way nav - #1706
Conversation
03968c6 to
a27c8e5
Compare
|
Love it! Initial response to the PR body:
Why double-click? I think single clicks would seem more logical? EDIT: Oh I think I see why you're doing double-click: to make a distinction between expand/collapse and "navigate to". Alternative suggestion: make the expand/collapse arrow icon have a larger clickable area, and make that its own click target. For people that want to only expand/collapse they can click the arrow icon, or they can click the full label to "expand and navigate to". Clicking the label should not collapse. |
rix0rrr
left a comment
There was a problem hiding this comment.
This is looking great!
Quick question, are the panes resizable?
| declare module 'prismjs/components/prism-core' { | ||
| import Prism from 'prismjs'; | ||
| export default Prism; | ||
| } | ||
| declare module 'prismjs/components/prism-json' {} | ||
| declare module 'prismjs/components/prism-yaml' {} | ||
| declare module 'prismjs/components/prism-clike' {} | ||
| declare module 'prismjs/components/prism-javascript' {} | ||
| declare module 'prismjs/components/prism-typescript' {} |
There was a problem hiding this comment.
What's this? Does Prism not come iwth its own type definitions?
There was a problem hiding this comment.
@types/prismjs only ships declarations for the top-level prismjs entry and the prismjs/components metadata module, not the individual prismjs/components/prism-* files. here im importing the deep paths on purpose, because prism-core is the minimal core, so we bundle only the languages we explicitly register instead of Prism's default auto-loaded set, plus one side-effect import per CDK language.
|
|
||
| async function getCachedAssembly(): Promise<AssemblyReadResult> { | ||
| const now = Date.now(); | ||
| if (cachedAssembly && (now - cachedAssembly.timestamp) < CACHE_TTL_MS) { |
There was a problem hiding this comment.
Is this the best invalidation we can do?
Why not look at the timestamp of manifest.json or something like that?
Also -- what about torn reads? I read some things from an old assembly, then it updates, and then I read some more things from a new assembly? And what happens to the read lock?
There was a problem hiding this comment.
ok so i took a look at this and basically entirely re did it. switched it to invalidate on manifest.json's mtime like you suggested.
each request stats manifest.json. if the mtime hasn't changed since the last read it serves the cached assembly and doesn't touch the lock at all. if it changed (or it's the first read) it re-reads under the read lock and caches against the new mtime. bonus: if a request comes in mid-synth before the manifest gets rewritten, the unchanged mtime just keeps serving the last complete generation instead of fighting for the lock.
on torn reads: the re-read holds the read lock so a synth can't rewrite cdk.out under it, and each cached entry is one whole generation. within a request the tree, violations and template index all come from that one snapshot so they can't be a mix of old and new. one thing to be clear about, the mtime isn't the guarantee (a synth writes a bunch of files and the manifest isn't necessarily written last), the lock is what guarantees a finished read, the mtime is just the cache key. the case still open is skew across separate requests (load /tree, a synth happens, then load /template).
on the read lock: there just wasn't one on the web path before, that was the actual gap. now it grabs the same lock the LSP uses (fromAssemblyDirectory().produce()). if the write lock is held it retries a bit then returns 503 instead of serving a half-written read, and a non-lock failure returns 500 instead of hanging.
heads up, sharing that lock meant moving the acquire helper out of lib/lsp into lib/core so i touched a couple LSP files too. kept those in their own commit and i'll open a matching LSP PR once this merges so the two branches base off each other cleanly.
Matching LSP side of #1706. Moves the assembly read-lock acquirer out of lib/lsp into lib/core/assembly-lock.ts so the LSP and web server build the read lock from one shared factory. Pure refactor, LSP behavior is unchanged, and server.ts re-exports AssemblyLock so existing importers keep resolving it. Fixes # ### Checklist - [ ] This change contains a major version upgrade for a dependency and I confirm all breaking changes are addressed - Release notes for the new version: --- By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license
…and template Rebased aws#1706 onto the updated feat/cdk-explorer (now carrying feat/cdk-lsp) as a single integration commit. Adds source/template/tree navigation with syntax highlighting, a YAML template view, and a file picker, and serves each assembly read under the Toolkit read lock via the factory core extracted in aws#1715. Coexists with the SSE live-refresh from aws#1698: the reload effect and the navigation state share the same App shell.
a27c8e5 to
379a422
Compare

A full cdk explore UI over the synthesized assembly. Builds on #1624.
Features
Design decisions
Checklist
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license