Skip to content
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

Desktop: Fixes #6719. Avoid reloading loaded plugins #6742

Merged
merged 1 commit into from
Aug 21, 2022

Conversation

SeptemberHX
Copy link
Contributor

This PR fixes #6719. It ignores the loaded plugins to avoid duplicate css.

Tested with the following plugins:

image

Before:
image

After:
image

@laurent22
Copy link
Owner

Looks good, thank you!

@laurent22 laurent22 merged commit 36871d9 into laurent22:dev Aug 21, 2022
@laurent22
Copy link
Owner

laurent22 commented Oct 31, 2022

@SeptemberHX, I think this change is now preventing content scripts from loading, when they should actually load. See this bug report there: https://discourse.joplinapp.org/t/pre-release-v2-9-is-now-available-updated-23-october/26416/57?u=laurent

The fact that the state is set outside the hook is a code smell anyway, and probably the source of this bug.

Do you have any suggestion on how to fix this?

@SeptemberHX
Copy link
Contributor Author

I reproduced this bug, and it works again after reverting this commit. Sorry for this problem.

After moving const loadedPluginIdSet = new Set<string>(); inside useExternalPlugins(), it still doesn't work. All the plugins needed to be reloaded after switching between notebooks and return from setting page, and the loadedPluginIdSet is also reset when initializing useExternalPlugins.

However, it seems all the js and others can be loaded without problem even when they are loaded, but css is not really necessary needed to be reloaded. And it works for me to remove

if (loadedPluginIdSet.has(contentScript.id)) {
	continue;
}

and move it to:

if (!loadedPluginIdSet.has(contentScript.id)) {
	if (mod.assets) {
		const cssStrings = [];
                 //...
	}
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Duplicate css properties for the assets of any codemirror plugins
2 participants