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

Update README.md #849

Open
wants to merge 2 commits into
base: 1.x
Choose a base branch
from
Open

Update README.md #849

wants to merge 2 commits into from

Conversation

dpfaffenbauer
Copy link

Restructure the plugin readme, I find it a bit confusing to find essential information at the end.

Also re the DllReference, please someone can comment on my comment? it does not work for me...

Copy link

sonarqubecloud bot commented Jan 9, 2025

@dpfaffenbauer
Copy link
Author

also the other examples don't really work... I get the error:

Init my plugin
main.ts:11 Start up my plugin
core-dll.js:2 Uncaught (in promise) TypeError: e.onInit is not a function
    at core-dll.js:2:1638256
    at Array.forEach (<anonymous>)
    at e.value (core-dll.js:2:1638233)
    at core-dll.js:2:2113377

@vin0401
Copy link
Collaborator

vin0401 commented Jan 14, 2025

Hi @dpfaffenbauer 👋

thanks for checking out the plugin system. :)
I agree with you - the npm installation should always comes first.

@vin0401
Copy link
Collaborator

vin0401 commented Jan 14, 2025

also the other examples don't really work... I get the error:

Init my plugin
main.ts:11 Start up my plugin
core-dll.js:2 Uncaught (in promise) TypeError: e.onInit is not a function
    at core-dll.js:2:1638256
    at Array.forEach (<anonymous>)
    at e.value (core-dll.js:2:1638233)
    at core-dll.js:2:2113377

The onInit-Method in this case should be related to the plugin-system, I guess.
But so far we can't really reproduce the error. Can you give us some additional information or can share some code with us, so that we can reproduce this error?

@dpfaffenbauer
Copy link
Author

@vin0401 here is what I did so far: dpfaffenbauer/CoreShop@1f96fac

@vin0401
Copy link
Collaborator

vin0401 commented Jan 16, 2025

@dpfaffenbauer - I was able to figure it out. :)

The error you send us is triggered due to this line:
dpfaffenbauer/CoreShop@1f96fac#diff-21fc732d543f3e01610a5a9dd7eba8b9a373cde2038f663acf4a40e5d9d71597R15

You are injecting a react component in this case but it should be the folderTabExtension-Module you created.
Also there are two cases where our docs are a bit outdated.

We renamed the name property to value:
dpfaffenbauer/CoreShop@1f96fac#diff-656b7a478de852d4b715106ac49f7e0ade545a002ef0930adf834970b7447b1fR20

And the openButtonWidget-method requires a config even tho it's not needed in that case:
dpfaffenbauer/CoreShop@1f96fac#diff-acdb71511c8fe828ede9157419f8118fd8ee883430f122b8db6b2f1bc88ea38dR9

       widgetManager.openBottomWidget({
            name: 'My first widget',
            component: 'my-first-widget',
            config: {}
        });

I will fix those cases in the next days in this PR with your changes included.
And will close it afterwards.

And maybe some additional information that could be interesting for you:

  1. The type interpretation of the npm package was not working, since we had a small issue in our package.json.
    This is fixed by now and should help you spot these errors.

  2. Somewhere in the weeks till inspire we will revisit the plugin system - there is no fixed date, yet.
    Than we will add a lot more of our UI components and and also more entrypoints for extension to our SDK.
    So, just that you know. :)

@dpfaffenbauer
Copy link
Author

❤️

Nothing more I can say :)

Re 2. We would be happy to assist in all sorts possible, I guess we have quite some plugins to migrate and we, together, can learn a lot and enhance a lot.

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.

2 participants