fix(typescript): revert #459 & #464

[oscard0m]

📝 Summary

Reverts the following PR's:

• fix(typescript): HandlerFunction argument #464
• fix(typescript): infer TTransformed from createEventHandler() options #459

⛱ Motivation and Context

Types for webhooks are broken when a transform function is passed in the options:
octokit/app.js#213 (comment)

Follow up conversation:
#492 (comment)

[wolfy1339]

This fixes the problem.

What about the scenario brought up in #464? Since we only allow to append to the event and not change it completely.

[oscard0m]

This fixes the problem.

I would like to add a test in typescript.validate.ts to cover the case we are fixing so we avoid regressions in the future

What about the scenario brought up in #464? Since we only allow to append to the event and not change it completely.

I would create an issue (I think there was not one before) so we keep discussing this. What do you think?

[wolfy1339]

I would like to add a test in typescript.validate.ts to cover the case we are fixing so we avoid regressions in the future

That should be as easy, just add an event handler for a certain event (ex: issues), and then try to access a property that exists only on that event (ex in the issues event: issue)

I would create an issue (I think there was not one before) so we keep discussing this. What do you think?

Yes, we should create an issue.

[oscard0m]

I would like to add a test in typescript.validate.ts to cover the case we are fixing so we avoid regressions in the future

That should be as easy, just add an event handler for a certain event (ex: issues), and then try to access a property that exists only on that event (ex in the issues event: issue)

We have this already, and seems it did not prevent us to break types:

webhooks.js/test/typescript-validate.ts

Lines 164 to 166 in 01c7655

	webhooks.on("issues", (event) => {
	console.log(event.payload.issue);
	});

This is the test example you were suggesting right?

[wolfy1339]

That doesn't have the transform method, but it is the exact sort of test that needs to be added. Look further down, there is another section of the tests where we have the transform method

webhooks.js/test/typescript-validate.ts

Lines 179 to 182 in 01c7655

	webhooks.on("issues", (event) => {
	// foo is set by options.transform
	console.log(event.foo);
	});

[oscard0m]

Ok, but with changes introduced by this PR it passes the transform method in L75 to all following lines so we are covering the use case now, right?

[wolfy1339]

After a second look, I can say that that should do the trick for the purposes of this PR

[oscard0m]

After a second look, I can say that that should do the trick for the purposes of this PR

Added a comment linking to this PR for that line to give context for other developers before modifying it in the future

[G-Rath]

Added a comment linking to this PR for that line to give context for other developers before modifying it in the future

Per my comment here, at some point we should modify the runtime code to prevent this as much as possible.

[wolfy1339]

Closes octokit/app.js#213