[sdk/nodejs] Fix a bug in closure serialization.

[swen128]

Description

When a closure captures an object which has complex properties (e.g. getters / setters), Pulumi serializes the properties using the defineProperty function.

This does not work properly if a function object has complex properties.
For example, when a class has a static getter, the getter is evaluated at deploy time and the returned value is used to serialize the class.

import {serializeFunction} from "@pulumi/pulumi/runtime";

class Class {
  static get foo() {
    return Math.random();
  }
}

serializeFunction(Class).then(f => {
  console.log(f.text);
});

The above script prints out the following serialized code (Notice the random value injected):

exports.handler = __f0;

__f0.foo = 0.7598867621294356;

function __f0() {
  return (function() {
    with({  }) {

return function /*constructor*/() { };

    }
  }).apply(undefined, undefined).apply(this, arguments);
}

Fixes #6908
The reported error occurs when Pulumi tries to serialize a getter BaseEntity.target

Checklist

• I have added tests that prove my fix is effective or that my feature works

• I have updated the CHANGELOG-PENDING file with my change

[github-actions]

PR is now waiting for a maintainer to run the acceptance tests.

Note for the maintainer: To run the acceptance tests, please comment /run-acceptance-tests on the PR

[github-actions]

PR is now waiting for a maintainer to run the acceptance tests.

Note for the maintainer: To run the acceptance tests, please comment /run-acceptance-tests on the PR

[kfirba]

Bravo!
Thanks for this PR! 👏🏻

[lukehoban]

/run-acceptance-tests

[github-actions]

Please view the results of the PR Build + Acceptance Tests Run Here

[github-actions]

Please view the results of the PR Build + Acceptance Tests Run Here

[github-actions]

Please view the results of the PR Build + Acceptance Tests Run Here

[kfirba]

@lukehoban Any plans on merging this? I'm stumbling across many issues with this, unfortunately

[github-actions]

PR is now waiting for a maintainer to run the acceptance tests.

Note for the maintainer: To run the acceptance tests, please comment /run-acceptance-tests on the PR

[lukehoban]

The one thing I'm slightly nervous about is that this change will cause a lot of usage of closure serialization to change - which will lead to e.g. redeploying all Lambdas when this version of Pulumi is adopted. That's not necessarily a blocker - though it is a downside we'll need to message.

I was initially wondering if this particular part of the change could be suppressed - but I do see that it is also technically incorrect to do what we were doing previously.

How would you characterize the cases where serialization would change after this PR? Is it "if you serialize a class definition, the serialization will be different"?

[swen128]

How would you characterize the cases where serialization would change after this PR? Is it "if you serialize a class definition, the serialization will be different"?

That characterization sounds good to me.

This PR could also affect any function whose property is defined via Object.defineProperty, but I don't think there are many use cases other than class definition.

[pgavlin]

@lukehoban @swen128 is this ready to go?

[github-actions]

PR is now waiting for a maintainer to run the acceptance tests.

Note for the maintainer: To run the acceptance tests, please comment /run-acceptance-tests on the PR

Further commands available:

• /run-codegen - used to test the Pull Request against downstream codegen
• /run-docs-gen - used to test the Pull Request against documentation generation

[lukehoban]

@lukehoban @swen128 is this ready to go?

Yes - I think we should merge this. It will lead to some magic functions being redeployed even with no user changes, but we've generally been okay with that in minor versions in the past.

[lukehoban]

/run-acceptance-tests

[github-actions]

Please view the results of the PR Build + Acceptance Tests Run Here

[github-actions]

PR is now waiting for a maintainer to run the acceptance tests.

Note for the maintainer: To run the acceptance tests, please comment /run-acceptance-tests on the PR

Further commands available:

• /run-codegen - used to test the Pull Request against downstream codegen
• /run-docs-gen - used to test the Pull Request against documentation generation