-
Notifications
You must be signed in to change notification settings - Fork 3.2k
Add plugin: Manuscript Timeline #5782
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Add plugin: Manuscript Timeline #5782
Conversation
Hello!I found the following issues in your plugin submission Errors: ❌ Your latest Release is missing the This check was done automatically. Do NOT open a new PR for re-validation. Instead, to trigger this check again, make a change to your PR and wait a few minutes, or close and re-open it. |
files added to repository. |
Hello!I found the following issues in your plugin submission Errors: ❌ Unable to find a release with the tag This check was done automatically. Do NOT open a new PR for re-validation. Instead, to trigger this check again, make a change to your PR and wait a few minutes, or close and re-open it. |
Published 1.0.3 release. |
Thank you for your submission, an automated scan of your plugin code's revealed the following issues: Required[1]:Please remove the [1]:The command name should not include the plugin name. [1][2][3][4][5][6][7][8][9][10][11][12][13][14][15][16][17][18]:You should avoid assigning styles via JavaScript or in HTML and instead move all these styles into CSS so that they are more easily adaptable by themes and snippets. [1][2]:Using Do NOT open a new PR for re-validation. |
Changes requested by bot have been made, ready for additional review by human. |
Hello!I found the following issues in your plugin submission Errors: ❌ Could not parse This check was done automatically. Do NOT open a new PR for re-validation. Instead, to trigger this check again, make a change to your PR and wait a few minutes, or close and re-open it. |
Hello!I found the following issues in your plugin submission Errors: ❌ The newly added entry is not at the end, or you are submitting on someone else's behalf. The last plugin in the list is: This check was done automatically. Do NOT open a new PR for re-validation. Instead, to trigger this check again, make a change to your PR and wait a few minutes, or close and re-open it. |
9882673
to
1286ed1
Compare
Hello!I found the following issues in your plugin submission Errors: ❌ Unable to find a release with the tag This check was done automatically. Do NOT open a new PR for re-validation. Instead, to trigger this check again, make a change to your PR and wait a few minutes, or close and re-open it. |
Thank you for your submission, an automated scan of your plugin code's revealed the following issues: Required[1][2][3][4][5][6][7][8][9][10][11][12][13][14][15][16][17][18][19][20][21][22][23][24][25][26][27]:You should avoid assigning styles via JavaScript or in HTML and instead move all these styles into CSS so that they are more easily adaptable by themes and snippets. [1][2][3][4][5][6][7][8][9][10][11][12][13][14][15][16][17][18][19][20][21][22][23][24][25][26][27][28][29][30] and more :You should consider limiting the number of Optional[1][2]:Casting to Do NOT open a new PR for re-validation. |
Changes requested by bot have been made, ready for additional review by human. |
this.plugin |
Hi Johannes,
Thank you for catching this. I've updated the code. Let me know if there is
anything else. Have a great weekend. -eric
this.plugin was replaced with
this as unknown as Component
<https://github.com/EricRhysTaylor/Obsidian-Manuscript-Timeline/blob/4238cc8c3c045b9776668968c6ee86cb0998f705/main.ts#L5405>
This change should resolve the memory leak issue.
Details:
1. **The Core Change:** The essential fix was changing the last argument
of `MarkdownRenderer.render` from `this.plugin` to `this`. This ensures
that the lifecycle of the rendered Markdown content (and any associated
event listeners created by the renderer) is tied to the
`ManuscriptTimelineSettingTab` instance itself.
2. **Lifecycle Management:** Since `ManuscriptTimelineSettingTab` extends
`PluginSettingTab`, which extends `Component`, Obsidian manages its
lifecycle. When the user closes the settings tab, Obsidian will unload this
specific component.
3. **Cleanup:** By passing `this` as the component context,
`MarkdownRenderer.render` registers its cleanup routines (like removing
event listeners) to be executed when *this specific settings tab component*
is unloaded.
4. **The Cast (`as unknown as Component`):** This part was purely to
satisfy the TypeScript compiler's type checking. It doesn't change the
underlying JavaScript behavior at runtime. We're still passing the settings
tab instance, which is what matters for fixing the leak.
So, when the settings tab is closed and its component is unloaded, the
resources used by the `MarkdownRenderer` for that specific instance should
now be properly released, preventing the memory leak.
…On Thu, Apr 24, 2025 at 12:11 PM Johannes Theiner ***@***.***> wrote:
*joethei* left a comment (obsidianmd/obsidian-releases#5782)
<#5782 (comment)>
this.plugin
<https://github.com/EricRhysTaylor/Obsidian-Manuscript-Timeline/blob/356654a8b357c23b7a05dae9bef9881bc0183758/main.ts#L5360>
The component passed to MarkdownRenderer.render should not be the plugin
instance.
Doing that leads to a memory leak, because new event listeners are
registered, but never removed.
The component is responsible for the lifecycle handling of the event
listeners related to the rendered result, so it should be unloaded once
there is nothing being rendered anymore.
—
Reply to this email directly, view it on GitHub
<#5782 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/BEFCPC6D2HP463YIRAGRIWD23EZPNAVCNFSM6AAAAABZRGL44CVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDQMRYGYYTQNRSGQ>
.
You are receiving this because you modified the open/close state.Message
ID: ***@***.***>
|
I am submitting a new Community Plugin
Repo URL
Link to my plugin: https://github.com/EricRhysTaylor/Obsidian-Manuscript-Timeline
Release Checklist
main.js
manifest.json
styles.css
(optional)v
)id
in mymanifest.json
matches theid
in thecommunity-plugins.json
file.I have given proper attribution to these other projects in my
README.md
.