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

Modify 'Must be attached' error to be more descriptive #376

Merged

Conversation

harryadel
Copy link
Contributor

This should fix #213

Now it outputs a more descriptive version: `${event} event triggerd with ${selector} on ${templateName} but associated view is not be found.
    Make sure the event doesn't destroy the view.`
Fixes meteor#213
@harryadel
Copy link
Contributor Author

harryadel commented Jun 19, 2022

cc @jankapunkt @StorytellerCZ
Along with #377 we can finally wrap up #371 and release it.

@@ -332,9 +332,11 @@ DOMRange.prototype.destroy = function (_skipNodes) {
DOMRange._destroy(this, _skipNodes);
};

DOMRange.prototype.containsElement = function (elem) {
DOMRange.prototype.containsElement = function (elem, selector, event) {
const templateName = this.view.name?.split('.')[1];
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would propose to add some fallback string in case we can't resolve the name:

const templateName = this.view?.name
  ? this.view.name.split('.')[1]
  : 'unknown template'

@harryadel
Copy link
Contributor Author

@denihs Can you grant @jankapunkt or me the ability to merge PRs instead of having to ask you?

@denihs
Copy link
Contributor

denihs commented Jun 20, 2022

@jankapunkt should be able to do it now. But to release a new version still have to be someone from inside. I can check this later with the team on how we can grant access to this part, in case @jankapunkt also wants to take care of releases.

@harryadel
Copy link
Contributor Author

Fine by me, thank you @denihs.

@jankapunkt jankapunkt added this to the Blaze 2.6.1 milestone Jun 20, 2022
@jankapunkt jankapunkt linked an issue Jun 20, 2022 that may be closed by this pull request
@jankapunkt jankapunkt merged commit e6bf934 into meteor:release-2.6.1 Jun 22, 2022
@jankapunkt jankapunkt mentioned this pull request Jul 13, 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.

Error handling, callbacks and DOMRange
3 participants