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

fix: Issue where there is only a single handler breaks #120

Closed
wants to merge 1 commit into from

Conversation

domtronn
Copy link

@domtronn domtronn commented Sep 1, 2017

This just casts handlers to an array so that the looping and invoking
always works

This just casts handlers to an array so that the looping and invoking
always works
@kbrsh
Copy link
Owner

kbrsh commented Sep 1, 2017

Huh, I don't think this is needed. Has there been any case where it is not an array?

@domtronn
Copy link
Author

domtronn commented Sep 1, 2017

Yes, I've been working with a child component which emits an event and a single parent app/view/component listening for it using

m-on:customEvent=handleEvent

With the handleEvent declared in the methods block. When the event fires, I get the following exception in the minified library

r[0] is not a function

I plugged in the unminified version and stepped through it and found that in this case the list of event handlers was just a single function. i.e. handlers === function (e) { } where that functions was my handleEvent function..

I'm not sure why it doesn't appear as a list, but this was the only way I was able to make it work and casting to an array seems benign?

@kbrsh
Copy link
Owner

kbrsh commented Sep 1, 2017

While it may seem harmless, it might actually be pointing to an underlying bug in Moon. Events should always be stored in an array.

Is it possible for you to provide a reproduction of the issue?

@kbrsh kbrsh closed this in f89d24e Sep 3, 2017
kbrsh added a commit that referenced this pull request Sep 7, 2017
- compiler
  - lexer
    - rewritten
    - ignores whitespace-only text nodes
    - can handle any whitespace (#127)
  - parser
    - rewritten
    - uses a stack to keep track of nodes
    - doesn't warn about extra closing tags
  - generator
    - cleaner implementation
    - cleaner specification for `state` and special directives
  - template compiler
    - rewritten
    - uses a single pass and eats leading/trailing whitespace
- directives
  - fix edge cases
  - beautify code more
- global API
  - remove "Moon.util.extend" (not used in core)
  - make component creation faster
- instance methods
  - event system
    - refactor ".on"
    - refactor ".emit"
  - mount
    - refactor
    - increase performance by caching
  - build
    - removed "patch" in favor of "build"
    - handles hydration, diffing, and replacing the root element
- observer
  - refactor
    - remove unused code
    - remove unused methods
  - computed properties
    - simpler caching system
      - when requested:
        - if cache is cleared:
          - captures dependencies for property
          - any ".get" will add the property as a dependency of the key
        - if not, use cached value
      - any ".set" notification will clear the cache for a key if it is not already cleared
  - methods
    - refactor loop
- DOM utilities
  - "appendChild" and "replaceChild" accept a vnode instead of a node to check for components in an efficient way (#132, #120)
    - "appendChild"
      - if component is found, create a dummy node and mount it
    - "replaceChild"
      - if component is found, mount it on the replacing node (will diff itself)
- utilities
  - remove "extend"
- virtual DOM
  - refactor
    - remove unused runtime utility
    - change code style
    - warn on missing directive
    - remove TEXT_TYPE constant
  - hydrate
    - efficiently hydrate children
  - diff
    - fix optimization case
  - use "dynamic" property to indicate deoptimization
  - replace "isSVG" with "SVG" meta property
kbrsh added a commit that referenced this pull request Sep 8, 2017
- compiler
  - lexer
    - rewritten
    - ignores whitespace-only text nodes
    - can handle any whitespace (#127)
  - parser
    - rewritten
    - uses a stack to keep track of nodes
    - doesn't warn about extra closing tags
  - generator
    - cleaner implementation
    - cleaner specification for `state` and special directives
  - template compiler
    - rewritten
    - uses a single pass and eats leading/trailing whitespace
- directives
  - fix edge cases
  - beautify code more
- global API
  - remove "Moon.util.extend" (not used in core)
  - make component creation faster
- instance methods
  - event system
    - refactor ".on"
    - refactor ".emit"
  - mount
    - refactor
    - increase performance by caching
  - build
    - removed "patch" in favor of "build"
    - handles hydration, diffing, and replacing the root element
- observer
  - refactor
    - remove unused code
    - remove unused methods
  - computed properties
    - simpler caching system
      - when requested:
        - if cache is cleared:
          - captures dependencies for property
          - any ".get" will add the property as a dependency of the key
        - if not, use cached value
      - any ".set" notification will clear the cache for a key if it is not already cleared
  - methods
    - refactor loop
- DOM utilities
  - "appendChild" and "replaceChild" accept a vnode instead of a node to check for components in an efficient way (#132, #120)
    - "appendChild"
      - if component is found, create a dummy node and mount it
    - "replaceChild"
      - if component is found, mount it on the replacing node (will diff itself)
- utilities
  - remove "extend"
- virtual DOM
  - refactor
    - remove unused runtime utility
    - change code style
    - warn on missing directive
    - remove TEXT_TYPE constant
  - hydrate
    - efficiently hydrate children
  - diff
    - fix optimization case
  - use "dynamic" property to indicate deoptimization
  - replace "isSVG" with "SVG" meta property
@ghost
Copy link

ghost commented Sep 8, 2017

Imo this issue grants a patch release. Components with events going up are completely broken in the stable branch v0.11.0. It took me an hour (just started using moon ;)) to figure out it was an error in the lib.

@kbrsh
Copy link
Owner

kbrsh commented Sep 8, 2017

It's actually a really hacky way of fixing it @renzo-s. The problem was that Moon was creating a DOM element for a component VNode before replacing it. This caused it to create a single handler for all of the event listeners instead of adding them.

For now, it would be best to try and use a global event emitter.

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