Skip to content

Conversation

@EmilianoSanchez
Copy link
Contributor

JavaScript commons library

What did you accomplish?

How do we test the changes introduced in this PR?

Extra Notes

@EmilianoSanchez EmilianoSanchez requested a review from a team as a code owner November 3, 2025 18:17
@EmilianoSanchez EmilianoSanchez changed the title Feat: add impressionsDisabled option to feature evaluation [WIP] Evaluate without impressions: add impressionsDisabled option to feature evaluation [WIP] Nov 3, 2025
@ZamoraEmmanuel ZamoraEmmanuel changed the title Evaluate without impressions: add impressionsDisabled option to feature evaluation [WIP] Evaluate without impressions: add impressionsDisabled option to feature evaluation Nov 19, 2025
@ZamoraEmmanuel ZamoraEmmanuel changed the base branch from development to ss-preset December 12, 2025 03:44
@ZamoraEmmanuel ZamoraEmmanuel changed the base branch from ss-preset to development December 12, 2025 03:45
@ZamoraEmmanuel ZamoraEmmanuel changed the base branch from development to ss-preset December 12, 2025 03:46
return properties && Object.keys(properties).length > 0 ? { properties } : undefined;
let options = properties && Object.keys(properties).length > 0 ? { properties } : undefined;

const impressionsDisabled = maybeOptions.impressionsDisabled;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We would need to refactor the validateEvaluationOptions so that the impressionsDisabled option is only validated and used for the Evaluator, and for other SDKs it should be sanitized to false (or any other falsy value) to ignore the feature.

Eventually, if the feature is included in SDKs, the change can be rolled back.

*
* @defaultValue `false`
*/
impressionsDisabled?: boolean;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We should delete this TypeScript definition, if the feature is not supported yet for SDKs.

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.

2 participants