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

fix: Acode ignoring main, readme and icon fields in plugin manifest #1085

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

alMukaafih
Copy link
Contributor

Fix to Acode Ignoring main, readme and icon fields in plugin manifest (#1026)

@bajrangCoder
Copy link
Collaborator

It fails to load existing plugins

@alMukaafih
Copy link
Contributor Author

It fails to load existing plugins

I will look into it

@bajrangCoder bajrangCoder marked this pull request as draft December 2, 2024 02:56
@bajrangCoder
Copy link
Collaborator

bajrangCoder commented Dec 6, 2024

I think problem is in plugin templates , as it points to main: "dist/main.js" but in zip file it's not in dist folder .

So handle this as legacy one

@alMukaafih
Copy link
Contributor Author

I think problem is in plugin templates , as it points to main: "dist/main.js" but in zip file it's not in dist folder .

So handle this as legacy one

I noticed this issue too. That is why I was testing for the existence of pluginJson.main before defaulting to "main.js" but it seems something went wrong somewhere.

@alMukaafih alMukaafih marked this pull request as ready for review December 27, 2024 15:46
@bajrangCoder bajrangCoder self-assigned this Dec 29, 2024
@bajrangCoder bajrangCoder added the enhancement New feature or request label Dec 29, 2024
@bajrangCoder
Copy link
Collaborator

The issue persists when loading previously published plugins; we need to implement a legacy solution to ensure compatibility.

@alMukaafih
Copy link
Contributor Author

The issue persists when loading previously published plugins; we need to implement a legacy solution to ensure compatibility.

I tried it on a clean install and it works. On an update it fails to load previously installed plugins.

@bajrangCoder
Copy link
Collaborator

To maintain compatibility, you can implement a simple check during the plugin load process. If the main field is set to dist/main.js, treat it as main.js instead. That's the main cause for problems.

@alMukaafih
Copy link
Contributor Author

To maintain compatibility, you can implement a simple check during the plugin load process. If the main field is set to dist/main.js, treat it as main.js instead. That's the main cause for problems.

I have implemented a more comprehensive check with my latest push. If the file that the main field points to does not exist, it falls back to the previous implementation of loading main.js .

@bajrangCoder
Copy link
Collaborator

Oo that's nice . But there are errors when listing local files in sidebar (I have tried your latest commit)

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

Successfully merging this pull request may close these issues.

2 participants