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

More package size reductions #2242

Merged
merged 4 commits into from
Mar 29, 2024
Merged

More package size reductions #2242

merged 4 commits into from
Mar 29, 2024

Conversation

noahdarveau-MSFT
Copy link
Contributor

@noahdarveau-MSFT noahdarveau-MSFT commented Mar 28, 2024

For more information about how to contribute to this repo, visit this page.

Description

Summarize the changes, including the goals and reasons for this change. Be sure to call out any technical or behavior changes that reviewers should be aware of.

If this Pull Request should close/resolve any issues when merged, use the special syntax for that here.

Main changes in the PR:

  1. I decided to go through the teams-js package and make as many enums const as possible. You can read the description for this PR for an explanation of why making enums const can when possible can make the package size smaller. I wasn't able to just go through all of the enums in teamsjs and blindly make them const since with the public enums, I have no way of knowing if the clients pulling the package are using the reverse mapping functionality (which making the enum const removes). However I was able to make most of the internal and private enums const as well as all of the public string enums, since in typescript string enums do not have the reverse mapping functionality anyways, so we don't have to worry about breaking it by making it const. There are also a few miscellaneous other enums that could not be made const due to other specific scenarios.

Package Size Reduction

  • About another 5% off the current main size (163,736 bytes from 170,924 bytes)

Validation

Validation performed:

  1. <Step 1>
  2. <Step 2>

Unit Tests added:

Unit tests are required for all changes. If no unit tests were added as part of this change, please explain why they aren't necessary.

<Yes/No>

End-to-end tests added:

<Yes/No>

Additional Requirements

Change file added:

Ensure the change file meets the formatting requirements.

<Yes/No>

Related PRs:

Remove this section if n/a

Next/remaining steps:

List the next or remaining steps in implementing the overall feature in subsequent PRs (or is the feature 100% complete after this?).

Remove this section if n/a

  • Item 1
  • Item 2

Screenshots:

Remove this section if n/a

Before After
< image1 > < image2 >

@noahdarveau-MSFT noahdarveau-MSFT requested a review from a team as a code owner March 28, 2024 23:03
jadahiya-MSFT
jadahiya-MSFT previously approved these changes Mar 28, 2024
Copy link
Contributor

@AE-MS AE-MS left a comment

Choose a reason for hiding this comment

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

:shipit:

@AE-MS AE-MS merged commit e67f1ed into main Mar 29, 2024
20 checks passed
@AE-MS AE-MS deleted the noahdarveau/more-const-enums branch March 29, 2024 15:54
alexneyman-MSFT pushed a commit that referenced this pull request Apr 17, 2024
* made all non-public and public string enums const

* removed const from some enums that cannot be const

* more enums that can't be const

* changefile
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.

None yet

3 participants