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

custom platforms can't use this #5131

Closed
skorfmann opened this issue Dec 5, 2023 · 1 comment · Fixed by #5412
Closed

custom platforms can't use this #5131

skorfmann opened this issue Dec 5, 2023 · 1 comment · Fixed by #5412
Labels
🐛 bug Something isn't working 🐶 dogfood Discovered while dogfooding Winglang 👠 platforms Issues relating to Wing Platform Providers

Comments

@skorfmann
Copy link
Contributor

I tried this:

export class Platform implements IPlatform {
  public readonly target = 'tf-aws';
  public readonly foo: string;

  constructor() {
    this.foo = 'bar';
  }

  postSynth(config: any) {
    console.log(this.foo);
    return config
  }
}

This happened:

this is not available

test/platform-production.test.ts > Platform > simple.main.w
Error: Cannot read properties of undefined (reading 'foo')
 ❯ runPreflightCodeInVm ../../../node_modules/.pnpm/@winglang+compiler@0.51.4_constructs@10.3.0/node_modules/@winglang/compiler/dist/index.js:468:11
 ❯ Proxy.compile ../../../node_modules/.pnpm/@winglang+compiler@0.51.4_constructs@10.3.0/node_modules/@winglang/compiler/dist/index.js:401:5
 ❯ test/platform-production.test.ts:31:22

I expected this:

either it should be made clear that the functions are called out of the class context or ideally this should just work as expected.

Is there a workaround?

don't use this

Anything else?

the hooks are collected here

let synthHooks: SynthHooks = {
preSynthesize: [],
postSynthesize: [],
validate: [],
};
let newInstanceOverrides: any[] = [];
this.platformInstances.forEach((instance) => {
if (instance.preSynth) {
synthHooks.preSynthesize!.push(instance.preSynth);
}
if (instance.postSynth) {
synthHooks.postSynthesize!.push(instance.postSynth);
}
if (instance.validate) {
synthHooks.validate!.push(instance.validate);
}
if (instance.newInstance) {
newInstanceOverrides.push(instance.newInstance);
}
});
return appCall!({ ...appProps, synthHooks, newInstanceOverrides }) as App;

that's where the hooks are called

if (this.synthHooks?.postSynthesize) {
this.synthHooks.postSynthesize.forEach((hook) => {
writeFileSync(
resolve(`${this.outdir}/main.tf.json`),
JSON.stringify(hook(tfConfig), null, 2)
);
});
}

Wing Version

0.51.7

Node.js Version

18.7

Platform(s)

MacOS

Community Notes

  • Please vote by adding a 👍 reaction to the issue to help us prioritize.
  • If you are interested to work on this issue, please leave a comment.
@skorfmann skorfmann added 🐛 bug Something isn't working 👠 platforms Issues relating to Wing Platform Providers 🐶 dogfood Discovered while dogfooding Winglang labels Dec 5, 2023
@mergify mergify bot closed this as completed in #5412 Jan 3, 2024
mergify bot pushed a commit that referenced this issue Jan 3, 2024
Since I was yanking out the callbacks for synth-hooks I needed to bind their associated instance to the callback's `this`

Closes: #5131

## Checklist

- [x] Title matches [Winglang's style guide](https://www.winglang.io/contributing/start-here/pull_requests#how-are-pull-request-titles-formatted)
- [x] Description explains motivation and solution
- [x] Tests added (always)
- [x] Docs updated (only required for features)
- [x] Added `pr/e2e-full` label if this feature requires end-to-end testing

*By submitting this pull request, I confirm that my contribution is made under the terms of the [Wing Cloud Contribution License](https://github.com/winglang/wing/blob/main/CONTRIBUTION_LICENSE.md)*.
@monadabot
Copy link
Contributor

Congrats! 🚀 This was released in Wing 0.54.28.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛 bug Something isn't working 🐶 dogfood Discovered while dogfooding Winglang 👠 platforms Issues relating to Wing Platform Providers
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants