search

eclipse-cdt-cloud / theia-trace-extension

Eclipse Theia trace viewer extension using the Trace Server Protocol (TSP), through the tsp-typescript-client. Also the home for reusable JavaScript libraries: traceviewer-base, traceviewer-react-components
MIT License
49 stars 60 forks source link

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

Open sgraband opened 12 months ago

sgraband commented 12 months ago

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 commented 12 months ago

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 commented 11 months ago

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 commented 11 months ago

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

[1] https://github.com/eclipse-cdt-cloud/theia-trace-extension/blob/a1d5a0cdb5ca00db86107e0f7847a76e20e70786/package.json#L131-L132