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

feat(angular): omit output related imports for components with no event #330

Merged
merged 2 commits into from
May 16, 2023

Conversation

KariiO
Copy link
Contributor

@KariiO KariiO commented Apr 21, 2023

Pull request checklist

Please check if your PR fulfills the following requirements:

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been reviewed and added / updated if needed (for bug fixes / features)
  • Build (npm run build) was run locally for affected output targets
  • Tests (npm test) were run locally and passed
  • Prettier (npm run prettier) was run locally and passed

Pull request type

Please check the type of change your PR introduces:

  • Bugfix
  • Feature
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • Documentation content changes
  • Other (please describe):

What is the current behavior?

It will always add EventEmitter and proxyOutputs imports event if there are no components with event.

Issue URL: Resolves #331

What is the new behavior?

Include import EventEmitter and proxyOutputs only if there is at least one component with event.

Does this introduce a breaking change?

  • Yes
  • No

Other information

@KariiO KariiO requested review from a team as code owners April 21, 2023 13:15
@liamdebeasi
Copy link
Contributor

Thanks for the PR! Is there an open issue on GitHub that you can link this to?

@KariiO
Copy link
Contributor Author

KariiO commented Apr 21, 2023

Unfortunately there is no issue for that - if its required I'll create one.

@liamdebeasi
Copy link
Contributor

Yes, please create one. We require PRs to be linked to triaged issues before they can be merged. Thanks!

@KariiO
Copy link
Contributor Author

KariiO commented Apr 21, 2023

Yes, please create one. We require PRs to be linked to triaged issues before they can be merged. Thanks!

Done - issue is created and linked.

Copy link
Contributor

@JessicaSachs JessicaSachs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM but would love another set of eyes on it from someone who knows Angular a bit better.

@liamdebeasi
Copy link
Contributor

@sean-perkins Mind taking a look when you get a chance?

@KariiO KariiO force-pushed the feat/angular-output-events branch 2 times, most recently from 53a9480 to f82db8c Compare May 10, 2023 12:51
@KariiO KariiO force-pushed the feat/angular-output-events branch from f82db8c to d3279d4 Compare May 10, 2023 19:50
Copy link
Collaborator

@sean-perkins sean-perkins left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me, thanks for this contribution and great work 🎉

@sean-perkins sean-perkins enabled auto-merge (squash) May 16, 2023 16:25
@sean-perkins sean-perkins enabled auto-merge (squash) May 16, 2023 16:26
@sean-perkins sean-perkins merged commit 4344797 into ionic-team:main May 16, 2023
3 checks passed
rwaskiewicz added a commit that referenced this pull request May 23, 2023
- feat(angular): omit output related imports for components with no events (#330)
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.

feat(angular): omit output related imports for components with no event #330
4 participants