Change interface to prototype chain

[An-dz]

Here's a first commit for implementing #176

Needs a lot of more testing, I guess it also allows #106

Todo

• You've added or updated the README.md if needed
• Your code has the necessary tests written
• You have updated the CHANGELOG.md unreleased sections

[chrisvanmook]

@An-dz Sorry for the delay, we're going to review this ASAP!

[An-dz]

By the way, it still lacks the react hooks to control some of the features through React.

[CoenWarmer]

Any movement on this? 😬

[An-dz]

Just waiting for a green light this is what's expected then I'll finish it.

[jonkoops]

Sorry for the delay on this. I'll be available next week to take a look!

[chrisvanmook]

Hi @An-dz

Sorry it took so long! We looked into your pull-request, however we do believe there are some issues regarding this solution that we tbh, in retrospect, could've also seen coming when opening issue #178.

1. We are forced to basically copy (a part) of the matomo's API: we need to know exactly all the options and add them into our API in order to use one. This will be very hard to maintain and will cause duplicate documentation eventually (matomo's and ours)
2. It should be possible to enable an option asynchonous, when for example fetching user info and id, then we are only able to set the ID after getting the response. This is not possible now as we need to set the userId on initialization.

I think we should make method available that can communicate directly with window._paq, that's also responsible of checking whether or not window._paq exists etc.

I'm curious what your opinion is on this.

[An-dz]

1. Yes, it's inevitable as the list can't be obtained otherwise. That's why I wrote Allow changing any Matomo configuration #174 with a specific object that allows anything, as I wrote there:

I wrote this to be able to pass setSecureCookie and setRequestMethod and noticed that it would not be optimal to write a specific case for each configuration possible.

For that reason I would change the interface to this:

instance
    .urlBase('https://LINK.TO.DOMAIN')
    .scriptPath('/js/matomo.js')
    .push('setSiteId', 3)
    .push('setUserId', 'example_id')
    .push('disableCookies')
    .push('setSecureCookie', true)
    .push('setRequestMethod', 'POST');

2. I believe the current code in this branch already allows that, I haven't tested but logically it should as the methods return the instance itself.

const MatomoInstance = new window.MatomoTracker
    .default('https://localhost')
    .setSiteId(3)

MatomoInstance.initialize()
// some code to get id
MatomoInstance.setUserId(uuid) // you can chain more stuff here too

[piotr-cz]

I would prefer if such methods would be optional to use.

For example I'm passing to MatomoTracker whole configuration object which is read from env file on build:

const matomo = new MatomoTracker(__MATOMO_OPTIONS__)

I don't have to check for optional configuration properties when some are available in ie. staging but not production

const matomo = new MatomoTracker(__MATOMO_OPTIONS__.urlBase)

matomo
  .setSiteId(__MATOMO_OPTIONS__.siteId)

if (___MATOMO_OPTIONS__.srcUrl !== undefined) {
  matomo.setSrcUrl(__MATOMO_OPTIONS__.srcUrl)
}

I mean fluent API is cool, but not for configurations.

[An-dz]

It wouldn't be a problem to implement a init convenience function to pass a configuration object to allow for @piotr-cz usage and still have a prototype interface for setting things after initial configuration.

[piotr-cz]

@An-dz Plus you wouldn't introduce a breaking change in API

[An-dz]

@piotr-cz I would, cause many of them are exactly problem number 1 @chrisvanmook mentioned.

For example: siteId, userId, linkTracking are redundant but at the same time different from Matomo API, I would get rid of them.

[jonkoops]

I am going to close this PR for now. We're looking to introduce a new API design in the future, once the core team agrees on a strategy.