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(NODE-4711)!: remove evalFunctions option #539

Merged
merged 10 commits into from
Dec 9, 2022
Merged

Conversation

nbbeeken
Copy link
Contributor

@nbbeeken nbbeeken commented Dec 7, 2022

Description

What is changing?

  • The evalFunctions option and friends are removed
  • deserialize always returns a Code instance
  • Migration guide is updated
  • Just for clarity: there are no changes to the serializeFunctions option

What is the motivation for this change?

Evaluating javascript strings is not a common practice and since the BSON code type is generally used for legacy operations (pre aggregation pipeline) it generally not javascript that would be useful to be invoked client side.

There is a migration example if this feature is still desired.

Double check the following

  • Ran npm run lint script
  • Self-review completed using the steps outlined here
  • PR title follows the correct format: <type>(NODE-xxxx)<!>: <description>
  • Changes are covered by tests
  • New TODOs have a related JIRA ticket

@durran durran added the Primary Review In Review with primary reviewer, not yet ready for team's eyes label Dec 8, 2022
test/node/parser/deserializer.test.ts Outdated Show resolved Hide resolved
test/node/parser/deserializer.test.ts Outdated Show resolved Hide resolved
src/code.ts Show resolved Hide resolved
@nbbeeken nbbeeken requested a review from durran December 8, 2022 17:59
durran
durran previously approved these changes Dec 8, 2022
@durran durran added Team Review Needs review from team and removed Primary Review In Review with primary reviewer, not yet ready for team's eyes labels Dec 8, 2022
durran
durran previously approved these changes Dec 9, 2022
docs/upgrade-to-v5.md Outdated Show resolved Hide resolved
@nbbeeken nbbeeken changed the title feat!(NODE-4711): remove evalFunctions option feat(NODE-4711)!: remove evalFunctions option Dec 9, 2022
@durran durran merged commit 0427eb5 into main Dec 9, 2022
@durran durran deleted the NODE-4711-rm-eval branch December 9, 2022 23:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Team Review Needs review from team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants