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

[API] Change enums to be const enum -- unless necessary to have the full enum name at runtime #1753

Open
1 of 2 tasks
MSNev opened this issue Dec 16, 2020 · 1 comment
Open
1 of 2 tasks

Comments

@MSNev
Copy link
Contributor

MSNev commented Dec 16, 2020

  • This only affects the JavaScript OpenTelemetry library
  • This may affect other libraries, but I would like to get opinions here first

To better support / reduce code size can we change all of the enum values to be const enum, unless there is an absolute need for reverse lookups at runtime from the enum "name" to it's value. And as part of the API no actual object results (like interfaces) in the final code.

In most cases this should be a non-breaking change, except for any reverse lookups. This should be done before GA to avoid anyone taking dependencies on the full name and creating their own reverse lookups based on the default const object created by TypeScript.

eg
(Would need to add explicit values to avoid accidental breaks from insertions in the future)

export const enum SamplingDecision {
NOT_RECORD,
RECORD,
RECORD_AND_SAMPLED
};

Usage becomes

if (sampling === /* SamplingDecision.RECORD */ 1) {
}

@MSNev
Copy link
Contributor Author

MSNev commented Dec 16, 2020

This looks like it's a potential duplicate of #538, but it should be done before GA

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

1 participant