Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
| <ul className=""> | ||
| {error.path.map((item) => ( | ||
| // NOTE: the \2192 here is the → character; tailwind needs it as an escape sequence. | ||
| <li className="after:content-['\2192'] after:ml-2 last:after:content-none" key={item}> |
There was a problem hiding this comment.
I guess this \ is ambiguous, so using \u makes it explicit
src/components/panels/problems.tsx
Outdated
| // NOTE: an index is appropriate here because a user could theoretically | ||
| // write a duplicate relationship, and the position makes some sense as a key | ||
| (invalid, index) => { |
There was a problem hiding this comment.
Check me on this
src/components/panels/terminal.tsx
Outdated
| if (!hasSelection) { | ||
| inputRef.current?.focus(); | ||
| const handleMouseUp = (event: MouseEvent) => { | ||
| if (event.target instanceof Element) { |
There was a problem hiding this comment.
This was interesting - target is typed as EventTarget rather than Element because technically the mouse event could come from anywhere within this div, and therefore the target isn't guaranteed to be an Element. A runtime check is the only way to guarantee it.
src/components/panels/terminal.tsx
Outdated
| escapeXML: true, | ||
| }); | ||
| const children = sections.flatMap((section: TerminalSection): ReactNode => { | ||
| const children = sections.flatMap((section, index) => { |
There was a problem hiding this comment.
Same here - this needed a key and I'm not sure there's anything more appropriate than the index
| // NOTE: we do not want to rerun this if the dataUpdated callback has changed (which it should | ||
| // not, ideally). | ||
| // TODO: dataUpdated is currently changing on every render because the debouncer isn't memoized. | ||
| // oxlint-disable-next-line eslint-plugin-react-hooks(exhaustive-deps) |
There was a problem hiding this comment.
Dealing with this later
src/components/ExamplesDropdown.tsx
Outdated
| }; | ||
| fetchExamples(); | ||
| if (examples === undefined) { | ||
| setExamples(LoadExamples()); |
There was a problem hiding this comment.
LoadExamples isn't a promise now - I'm not sure when that changed
| }; | ||
|
|
||
| const conductUpload = () => { | ||
| (async () => { |
There was a problem hiding this comment.
This is getting rid of the async wrapper and pushing it up onto conductUpload
src/components/FullPlayground.tsx
Outdated
| }); | ||
| return; | ||
| } | ||
| const contents = await file.text(); |
There was a problem hiding this comment.
This is what lets us get rid of getFileContentsAsText - file now implements Blob which means we can just await .text()
src/spicedb-common/parsing.ts
Outdated
|
|
||
| contextualizedCaveat.context = caveatContext; | ||
| } catch (e) { | ||
| const errorText = e instanceof Error ? e.message : ""; |
There was a problem hiding this comment.
e was previously treated as any,which is technically incorrect. This fixes that.
cb31a37 to
002ee71
Compare
I screwed up the rebase on this one. See #119