Repository navigation
ENG-2312 Release mobile-supported DG obsidian - #1533
Open
trangdoan982 wants to merge 7 commits into
Open
trangdoan982 wants to merge 7 commits into
trangdoan982 wants to merge 7 commits into
Conversation
gray-matter requires `fs` and mime-types requires `path` at module load, so both throw at plugin load on Obsidian mobile. Replace them with local utilities and stop marking Node builtins external, so a future Node import fails the build instead of shipping. The mime table is generated from mime-db rather than curated: the caller skips every `text/` attachment, so an extension missing from it would silently start uploading transcluded notes as binary assets. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Entire-Checkpoint: 01M4E8D4VYPYHG3FRYDTXXG58A
parseFrontmatter now calls parseYaml, which the stub did not export, so importRelations' tests read every frontmatter as empty. Obsidian's parseYaml is `YAML.parse` from the `yaml` package, so the stub delegates to it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Entire-Checkpoint: 01M4EK4RJSMRSJP7DA5VCAXSWG
The engine swap and the swallowed parse error had no tests of their own, and the comment on the markdown-link replacer pointed at the image pass as being above it when it runs below. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Entire-Checkpoint: 01M4EK777W55T20BNRYMHXH4RD
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
An attachment named `x.constructor` read an inherited member out of the mime table, and the caller's `.startsWith` on it threw and ended the node's asset sync. `mime-types` returned the default here, so this was a regression. Null-prototyping the table makes the miss structural. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Entire-Checkpoint: 01M4H0QVME2ME0S3ENM6X7V6AP
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Entire-Checkpoint: 01M4H7Q15TT8N88A88452CJCZE
This branch was successfully deployed
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
1 participant
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.
Reviewer brief
1.7.1-beta.2prerelease. Desktop behaviour is unchanged.apps/obsidian/src/utils/mimeType.ts. Its caller skips everytext/attachment, so an extension missing from the table silently starts uploading transcluded notes as binary assets. The table is generated from mime-db rather than hand-curated for that reason: a hand-curated table left out 92text/extensions, so.markdown,.mdxand.htmembeds would have started uploading, and it typed three more wrongly — including.xml, which flipped from uploaded to skipped.process.envunguarded and would throw on mobile on that path only; pre-existing, not touched here.Two review findings are accepted rather than fixed, recorded here so they are decisions and not surprises:
docx,zip,epub,heic, fonts) is now stored withapplication/octet-streamas its Supabase content type instead of a specific one. Nothing stops uploading, since the skip gate istext/only, and Obsidian does not render these inline. Covering every type would mean a 41.5 KB table in place of 3.9 KB.Platform.isMobileguard, which is the first of the touch-affordance work and belongs with it.Verification
Equivalence with the replaced packages was established by differential testing against the real implementations, not by inspection:
mimeType.tsvsmime-typesacross all 1,239 known extensions: 0 upload/skip decision changes; every Obsidian attachment type byte-identical.splitFrontmatter+parseYamlvsgray-matteracross 394 real vault notes: 0 frontmatter-presence diffs, 0 body diffs, and 0 diffs innodeTypeId/importedFromRid, the only fields any caller reads. Ground truth came from Obsidian's ownmetadataCache.!![x], escaped bang, newline-separated bang, nested brackets): 0 differences.require("fs")orrequire("path"), zero lookbehind assertions.pnpm check-types,pnpm lint(0 errors),pnpm test:unit(181 passed).1.7.1-beta.2on Obsidian mobile and confirmed it loads.Two behaviour changes are deliberate and worth a reviewer's eye:
Date. Obsidian'sparseYamlis theyamlpackage, not js-yaml, so this makes the plugin agree withmetadataCache. No caller reads a date field.Loom video
Outstanding — to be added before review.
Scope check
$scope-checkagainst ENG-2312 and the final diff.Done When: None.Standards check
$dg-pr-adherence-checkagainst the final diff and PR metadata.Local delegated full review
$dg-delegated-full-reviewwhen no other full-review workflow is available.