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

Cleanup framework angular dependencies #19389

Merged
merged 8 commits into from
Oct 17, 2022

Conversation

kroeder
Copy link
Member

@kroeder kroeder commented Oct 7, 2022

What I did

I noticed that the framework/angular/package.json needed a cleanup.

For checking missing or obsolete dependencies, I used npx depcheck code/frameworks/angular

How to test

  • Is this testable with Jest or Chromatic screenshots?
  • Does this need a new example in the kitchen sink apps?
  • Does this need an update to the documentation?

If your answer is yes to any of these, please make sure to include it in your PR.

@kroeder kroeder added maintenance User-facing maintenance tasks angular labels Oct 7, 2022
@kroeder kroeder self-assigned this Oct 7, 2022
@kroeder kroeder requested a review from ndelangen October 7, 2022 10:27
@ndelangen
Copy link
Member

I think right now.. the dependency on react can't be removed... I think?

@kroeder
Copy link
Member Author

kroeder commented Oct 7, 2022

Who is responsible for providing a react-version in this mono-repo?

Nothing references react in frameworks/angular. I assumed that react is provided by a different package that needs react installed.

I can undo that, though. Any way to test if this change is ok or not?

@kroeder
Copy link
Member Author

kroeder commented Oct 7, 2022

The test runner figured out that something is missing :) I get the tests green first, lets discuss further changes aftwards

@kroeder kroeder force-pushed the framework-angular-cleanup-dependencies branch from 3208215 to 4ec4e23 Compare October 7, 2022 20:01
@kroeder
Copy link
Member Author

kroeder commented Oct 7, 2022

@ndelangen Since I'm not sure whether we need @types for react as dep or not, I re-added them.

For the rest, here's a quick summary

All of these are not guaranteed to be correct but I don't know how to find out other than looking for references in code files.

@kroeder
Copy link
Member Author

kroeder commented Oct 13, 2022

@ndelangen do you need anything else?

According to `npx depcheck`, the dependencies
in this commit can be removed.
The dependency @storybook/core-webpack is being referenced
in angular/src/types.ts but not listed as dependency.
@kroeder kroeder force-pushed the framework-angular-cleanup-dependencies branch from 4ec4e23 to 585596c Compare October 15, 2022 10:07
@ndelangen ndelangen merged commit 53dacb3 into next Oct 17, 2022
@ndelangen ndelangen deleted the framework-angular-cleanup-dependencies branch October 17, 2022 20:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
angular maintenance User-facing maintenance tasks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants