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.

Sign up for GitHub

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

Jump to bottom

can-component beforeremove-event does not fire with [email protected] #178

Open
cleong-tc opened this issue Jun 26, 2018 · 6 comments
Open

can-component beforeremove-event does not fire with [email protected] #178

cleong-tc opened this issue Jun 26, 2018 · 6 comments

Comments

@cleong-tc
Copy link
Contributor

cleong-tc commented Jun 26, 2018 •
edited
Loading

[email protected] added Node.prototype.removeEventListener to register.js which caused can-component/can-control beforeremove-event to not fire.
https://v3.canjs.com/doc/can-component/beforeremove.html

it seems the beforeremove-event-handlers are being removed by Node.prototype.removeEventListener() (in can-simple-dom/lib/simple-dom/event.js) before Node.prototype.dispatchEvent() can call the beforeremove-event-handlers. Node.prototype.removeEventListener() is removing the beforeremove-event-handlers because if (handlersByType[index] === handler) { evaluates to true for [email protected] (vs false for [email protected]).

in my donejs@1 sample app, with [email protected] the log-message for the beforeremove-event is displayed in the terminal-console, but with [email protected] the log-message is missing. we use the beforeremove-event to cleanup timers and element-event-bindings.

src/index.stache...

<html>
  <head>
  </head>
  <body>
		<can-import from="my-app/app" export-as="viewModel" />
		<can-import from="my-app/mobile/index/" />
		<mobile-index />

    {{#switch env.NODE_ENV}}
      {{#case "production"}}
        <script src="{{joinBase 'steal.production.js'}}"></script>
      {{/case}}
      {{#default}}
        <script src="/node_modules/steal/steal.js"></script>
      {{/default}}
    {{/switch}}
  </body>
</html>

src/mobile/index/index.js...

import Component from 'can-component';
import DefineMap from 'can-define/map/';
import view from './index.stache';

export const ViewModel = DefineMap.extend({
});

export default Component.extend({
  tag: 'mobile-index',
  ViewModel,
	view,
	events: {
		'inserted'() {
			console.log('mobile-index.events.inserted');
		},
		'removed'() {
			console.log('mobile-index.events.removed');
		},
		'{element} beforeremove'() {
			console.log('mobile-index.events.element.beforeremove');
		},
	}
});

package.json...

  "dependencies": {
    "commander": "^2.8.1",
    "debug": "^2.6.3",
    "express": "^4.13.4",
    "http-proxy": "^1.13.2",
    "can-component": "^3.3.4",
    "can-define": "^1.4.4",
    "can-route": "^3.3.4",
    "can-route-pushstate": "^3.1.0",
    "can-set": "^1.3.2",
    "can-stache": "^3.6.2",
    "can-view-import": "=3.2.1",
    "can-view-autorender": "^3.1.1",
    "can-util": "^3.9.10",
    "can-zone": "=0.6.13",
    "done-autorender": "^1.2.0",
    "done-component": "^1.0.0",
    "done-css": "^3.0.1",
    "done-ssr": "=1.1.5",
    "done-ssr-middleware": "=1.1.0",
    "steal": "^1.12.3",
    "steal-less": "^1.3.1",
    "steal-stache": "^3.1.2",
    "steal-tools": "^1.11.9"
  },
  • note: there is a version-lock for [email protected] to resolve a memory-leak-issue because of can-view-import-promise.
@matthewp
Copy link
Contributor

Thanks, I'll see if I can recreate.

@matthewp
Copy link
Contributor

matthewp commented Jun 26, 2018 •
edited
Loading

I created a glitch: https://glitch.com/edit/#!/sophisticated-turkey?path=server.js:13:0

What I see there seems to work:

screen shot 2018-06-26 at 4 50 40 pm

Using your deps list. What's different?

@cleong-tc
Copy link
Contributor Author

@matthewp : in package.json change "can-zone": "=0.6.13" to "can-zone": "=0.6.12" and you should see the additional terminal-console-message mobile-index.events.element.beforeremove.

sorry, i am not familiar with glitch.com. i need to figure out how to see the console-output.

@matthewp
Copy link
Contributor

Ah, I see. Interesting... ok, this gives me something to work with so I'll check it out.

@cleong-tc cleong-tc mentioned this issue Aug 2, 2018
Closed
@cleong-tc
Copy link
Contributor Author

@matthewp : beforeremove-event seems to be firing in done-ssr@2, so i am fine with ignoring this issue.

@matthewp
Copy link
Contributor

matthewp commented Aug 3, 2018

Hm, so probably not a bug in can-zone but rather more likely in done-ssr. Trying to think if we had tests with beforeremove...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@matthewp @cleong-tc