Skip to content

Fixes for observe method#74

Open
lekhmanrus wants to merge 3 commits intozoomsphere:masterfrom
lekhmanrus:master
Open

Fixes for observe method#74
lekhmanrus wants to merge 3 commits intozoomsphere:masterfrom
lekhmanrus:master

Conversation

@lekhmanrus
Copy link
Copy Markdown

  • returned event, not events array
  • fixed condition for exact match keys
  • removed unused imports

Copy link
Copy Markdown
Author

@lekhmanrus lekhmanrus left a comment

Choose a reason for hiding this comment

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

My mistake. don't apply commit b3b175a. sorry

if (!key) { return true; }
if (exactMatch) {
if (key.startsWith(Config.prefix)) {
if (!key.startsWith(Config.prefix)) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why? It doesn't make sens to add Config.prefix to the key below if the key already starts with prefix...

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Tbh, I don't remember exactly why I did that... but I suppose, it is because event.key is a key without prefix, and that fix is to observe key without prefix, so, for example:
Before:

if (exactMatch) {
  if (key.startsWith(Config.prefix)) {
  // if ('test'.startsWith('im.')) { // false
    return event.key === key;
  }
  return event.key === Config.prefix + key;
  // return 'test' === 'im.' + 'test'; // false
}

After:

if (exactMatch) {
  if (!key.startsWith(Config.prefix)) {
  // if (!'test'.startsWith('im.')) { // true
    return event.key === key;
    // return 'test' === 'test'; // true
  }
  return event.key === Config.prefix + key;
}

But it won't work for key with prefix in both cases - Example 2:
Before:

if (exactMatch) {
  if (key.startsWith(Config.prefix)) {
  // if ('im.test'.startsWith('im.')) { // true
    return event.key === key;
    // return 'test' === 'im.test'; // false
  }
  return event.key === Config.prefix + key;
}

After:

if (exactMatch) {
  if (!key.startsWith(Config.prefix)) {
  // if (!'im.test'.startsWith('im.')) { // false
    return event.key === key;
  }
  return event.key === Config.prefix + key;
  // return 'test' === 'im.' + 'im.test'; // false
}

But I don't remember exactly. It was a while ago.

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