Skip to content

Conversation

@bartoszglow
Copy link
Contributor

@bartoszglow bartoszglow commented Sep 22, 2020

Closes #100
Closes #91

This PR:

  • makes the data layer always behave the same: it does not matter whether the custom code is executed before or after the data layer script has loaded
  • makes the callback function fire just once, whether registering an event listener before or after adobe-client-data-layer.min.js has loaded
  • adds tests to verify the behavior
  • adds tests to verify nested anonymous functions behaviour

Examples:

  1. Event listener with scope set to past, late ACDL initialization:
  adobeDataLayer.push(function(adobeDataLayer) {
    adobeDataLayer.addEventListener('change', function(event) { ... }, { scope: 'past' });
  });

  adobeDataLayer.push({
    event: 'change'
  });

  // ACDL gets initialized afterwards

Result: event listener will not be triggered

  1. Event listener with scope set to future, late ACDL initialization:
  adobeDataLayer.push(function(adobeDataLayer) {
    adobeDataLayer.addEventListener('change', function(event) { ... }, { scope: 'future' });
  });

  adobeDataLayer.push({
    event: 'change'
  });

  // ACDL gets initialized afterwards

Result: event listener will be triggered

  1. Event listener with scope set to past, early ACDL initialization:
  // ACDL gets initialized before

  adobeDataLayer.push(function(adobeDataLayer) {
    adobeDataLayer.addEventListener('change', function(event) { ... }, { scope: 'past' });
  });

  adobeDataLayer.push({
    event: 'change'
  });

Result: event listener will not be triggered

  1. Event listener with scope set to future, late ACDL initialization:
  // ACDL gets initialized before

  adobeDataLayer.push(function(adobeDataLayer) {
    adobeDataLayer.addEventListener('change', function(event) { ... }, { scope: 'future' });
  });

  adobeDataLayer.push({
    event: 'change'
  });

Result: event listener will be triggered

@jckautzmann
Copy link
Contributor

jckautzmann commented Sep 22, 2020

We should have new tests to verify the correct behaviour as described in #100:

  1. add event listener before DL initialization -> add event -> verify that the listener is triggered as soon as the event is processed
  2. add event listener after DL initialization -> add event -> verify that the listener is triggered right away

@codecov
Copy link

codecov bot commented Sep 23, 2020

Codecov Report

Merging #101 into master will decrease coverage by 0.67%.
The diff coverage is 91.66%.

Impacted file tree graph

@@             Coverage Diff             @@
##            master     #101      +/-   ##
===========================================
- Coverage   100.00%   99.32%   -0.68%     
===========================================
  Files           12       11       -1     
  Lines          302      295       -7     
  Branches        69       66       -3     
===========================================
- Hits           302      293       -9     
- Misses           0        2       +2     
Impacted Files Coverage Δ
src/listenerManager.js 100.00% <ø> (ø)
src/dataLayerManager.js 98.27% <90.90%> (-1.73%) ⬇️
src/utils/dataMatchesContraints.js 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ae59430...b2e6244. Read the comment docs.

bartoszglow and others added 11 commits September 24, 2020 11:58
- add test to test the behaviour:
before/after script load
scope = future/past
- POC: introduce an array with the items that have been added to the data layer array before the DL script had loaded
- pushing each item into the data layer array (_datalayer) will process the item by using the augmented push method so the behaviour should be the same as when the DL script is loaded
@bartoszglow bartoszglow merged commit a156b68 into master Oct 7, 2020
@bartoszglow bartoszglow deleted the bugfix/event-listener-callback-issue branch October 7, 2020 15:02
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.

[Regression] getState method returns no data in event callback Inconsistent behaviour when registering event listeners

3 participants