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

Add dependencies from root as peer-dependencies in extension #1017

Open
sgraband opened this issue Sep 18, 2023 · 3 comments
Open

Add dependencies from root as peer-dependencies in extension #1017

sgraband opened this issue Sep 18, 2023 · 3 comments

Comments

@sgraband
Copy link
Contributor

While updating the CDT Cloud blueprint to the latest version of this extension i noticed that some dependencies do not seem to be provided correctly.

To be more specific: It seems like these dependencies from the root package.json are only references as dependencies there, but not in the theia-traceviewer extensions package.json.

Therefore, i need to manually add these dependencies to my project, when i want to consume the theia-traceviewer extension.

These dependencies should probably be added as peer dependencies in the theia-traceviewer package.json. Or was there a specifiic reason for it to be implemented this way?

@bhufmann
Copy link
Collaborator

bhufmann commented Sep 18, 2023

While updating the CDT Cloud blueprint to the latest version of this extension i noticed that some dependencies do not seem to be provided correctly.

To be more specific: It seems like these dependencies from the root package.json are only references as dependencies there, but not in the theia-traceviewer extensions package.json.

Therefore, i need to manually add these dependencies to my project, when i want to consume the theia-traceviewer extension.

These dependencies should probably be added as peer dependencies in the theia-traceviewer package.json. Or was there a specifiic reason for it to be implemented this way?

No, I don't think there was a specific reason for that. When the change was introduced I think it was not noticed or not questioned. If you think that's incorrect, could you please provide a PR and we will review it.

Thanks for bringing this up.

@marcdumais-work
Copy link
Contributor

I believe that this is a legit issue - the way these production dependencies are defined ATM, they will be available to the test application but not be included in the published Theia extension. If they are only needed in the extension, I think they should be moved there.

@marcdumais-work
Copy link
Contributor

The two related @types dependencies [1] should probably be moved to the extension too, in devDependencies.

[1]

"@types/react": "^18.0.15",
"@types/react-dom": "^18.0.6"

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

No branches or pull requests

3 participants