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

Deprecate V1 closure serializer and default to V2 #512

Open
3 tasks
sam-goodwin opened this issue Sep 16, 2022 · 1 comment
Open
3 tasks

Deprecate V1 closure serializer and default to V2 #512

sam-goodwin opened this issue Sep 16, 2022 · 1 comment
Assignees
Labels

Comments

@sam-goodwin
Copy link
Collaborator

With the release of #402, we now have two closures serializeres.

  • V1 - closure serializer based off of Pulumi's closures serializer. It uses the debugger API and is heavily limited, for example not properly serializing free variables such that mutations can be observed, cannot capture this, cannot serializer Proxies or bound functions, and has no source map support and therefore produces code that is not production worthy. The code for this serializer is also a spaghetti nightmare, making it impossible to maintain. Finally, the implementation is extremely SLOW and asynchronous, making it a bad fit for use within CDK Constructs.
  • V2 - our own serializer that takes advantage of the ast-reflection pre-processing step. We decorate all of a user's code with its AST information to support features such as IAM policy inference and the Functionless interpreteres. We now also use this information to provide a fast and synchronous closure serializer with out any of the aforementioned limitations. Our ast-reflection package can always be updated to inject more information to help support the serializer, meaning we are not constrained by the limitations of the V8 debugger API.

As of now, we still default to the V1 serializer - users must opt-in to use V2 to get all of its benefits. This task is to go through the process of thoroughly testing the new serializer so that we are confident in moving over to it first class. V2 has been tested against all of the test cases in functionless but this does not give us the confidence we need.

This task can be achieved int he following steps:

  • thoroughly explore test cases for the new serializer in serializer-cloures.test.ts - tests are pretty good but not thorough enough yet
  • update the default serializer to V2 but still allow users to opt-out and go back to V1
  • once we are confident enough, remove V1, delete the SerializerImpl enum, deprecate the @functionless/nodejs-closure-serializer package and delete the repository/change it to private.
@thantos
Copy link
Collaborator

thantos commented Sep 20, 2022

Could you add a statement about the state of source maps?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants