Skip to content

Conversation

@greghuels
Copy link
Contributor

@greghuels greghuels commented Dec 24, 2025

What does this PR do? What is the motivation?

The Node.js docs for OpenFeature suggested using OpenFeature.setContext to set user-specified attributes. This is not a good practice for server SDKs since it leads to race conditions. Examples in the docs have been updated to evaluation-specific context.

Merge instructions

Merge readiness:

  • Ready for merge

@github-actions
Copy link
Contributor

Preview links (active after the build_preview check completes)

Modified Files

Copy link
Member

@dd-oleksii dd-oleksii left a comment

Choose a reason for hiding this comment

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

The change looks good. However it looks like targeting key is missing, so the sharding will not work properly and I believe that's not intended

Comment on lines 76 to 79
const evaluationContext = {
userID: req.session?.userID,
companyID: req.session?.companyID
});
};
Copy link
Member

Choose a reason for hiding this comment

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

shall we set targetingKey here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, we definitely should. I'll use userID as the targeting key

Copy link
Contributor

@evazorro evazorro left a comment

Choose a reason for hiding this comment

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

I'll let other reviewers comment on the technical accuracy, but this looks good from a Markdown formatting perspective so I'm approving! Let me know if you need a re-approval after applying others' suggestions.

@greghuels greghuels merged commit 6ffc813 into master Dec 24, 2025
16 checks passed
@greghuels greghuels deleted the greg.huels/ff-nodejs-context-update branch December 24, 2025 15:23
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.

4 participants