UncleGedd commented on pull request #64: URL: https://github.com/apache/incubator-flagon-useralejs/pull/64#issuecomment-804210909
Hi all, while I was implementing the proposed changes I came across a curious behavior. Currently, `packageCustomLog` does not take the event as an argument and, as a result, does not pass the event to the `mapHandler` function: https://github.com/apache/incubator-flagon-useralejs/blob/7a7b2ce73963d9dd1b0bff9419ded12e15af10ad/src/packageLogs.js#L165-L166 However, in `packageLog` we do take the event as an argument and pass it to the `mapHandler`: https://github.com/apache/incubator-flagon-useralejs/blob/7a7b2ce73963d9dd1b0bff9419ded12e15af10ad/src/packageLogs.js#L116-L118 Not passing the event into `packageCustomLog` can lead to errors when using `userale.map` and `userale.packageCustomLog` together. In the [example](https://github.com/apache/incubator-flagon-useralejs/pull/64#discussion_r597152859) mentioned in Rob's comment above, we check the properties of the `e` argument to determine if we need to add custom labeling. However, if you setup the `userale.map` function this way and call `packageCustomLog`, then the `e` argument never gets passed to the `window.userale.map` function, resulting in an error when `e` properties are accessed as `e` does not exist. In the latest commit to this PR I get around this issue by checking for `e` before accessing its properties, but this feels a bit hacky. If I'm understanding everything correctly, we should probably either add `e` as an argument to the mapHandler in `packageCustomLog` or provide a way to bypass the mapHandler in that function. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: [email protected]
