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

Add type check for eventProxy function #142

Closed
wants to merge 2 commits into from

Conversation

AkeemMcLennon
Copy link

Very that event proxy callback function is actually a function

@developit
Copy link
Member

Hi Akeem,

Any context for this PR? I had left out the type check in order to show an error when the handler was the wrong type. Though I guess the error would be a bit useless ("cannot call method call of undefined"). Seems like it would be worth showing some form of error though? Interested to hear your thoughts.

@AkeemMcLennon
Copy link
Author

Hi @developit, this really goes in conjunction with developit/preact-markup#3

When I wrote a test for the negative case where the allow-events weren't enabled, I got the undefined error.

However, I'm fine with throwing an Error / Exception instead if that's what you'd prefer.

@developit
Copy link
Member

Ah, excellent, that's what I was looking for. I'm going to check if there is a way to throw a more useful error here, will let you know. Part of an ongoing effort to provide a better debugging experience, but retain the dev <-> prod parity, instead of requiring that people switch builds or apply globals to get suitably production-worthy builds.

@AkeemMcLennon
Copy link
Author

Updated to throw an Error now

@developit
Copy link
Member

I think we might be able to combine this with #153. If it can cover all the cases for event handlers, need add more tests and look at compat, then I believe the TypeError would be similar to this one. Will keep you posted as I merge these on Thursday/Friday (sorry for vacation delays!).

@developit
Copy link
Member

Hey Akeem - I'm going to close this one out since the work for #153 ended up throwing a TypeError here for non-function handlers. Thanks so much for your work!!

@developit developit closed this May 29, 2016
marvinhagemeister added a commit that referenced this pull request Mar 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants