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

Importing MatomoRouterModule results in Error retrieving getBaseHrefFromDOM #82

Closed
Viserius opened this issue Jan 30, 2024 · 7 comments · Fixed by #83
Closed

Importing MatomoRouterModule results in Error retrieving getBaseHrefFromDOM #82

Viserius opened this issue Jan 30, 2024 · 7 comments · Fixed by #83
Labels
bug Something isn't working released

Comments

@Viserius
Copy link
Contributor

Dear @EmmanuelRoux,

Adding MatomoModule to the module imports has been working for us so far in our Angular 17 application.
Since we now want to do some magic on setting a custom URL before sending results to the server, we tried adding the MatomoRouterModule to the imports as well.
Even before we add any custom configuration such as an interceptor, adding the MatomoRouterModule to our imports causes the following error to take place on a page load:

ERROR TypeError: Cannot read properties of null (reading 'endsWith')
    at trimTrailingSlash (ngx-matomo-client-router.mjs:178:16)
    at DefaultPageUrlProvider.getBaseHrefWithoutTrailingSlash (ngx-matomo-client-router.mjs:193:16)
    at DefaultPageUrlProvider.getCurrentPageUrl (ngx-matomo-client-router.mjs:188:20)
    at MatomoRouter.presetPageTitleAndUrl (ngx-matomo-client-router.mjs:271:14)
    at ngx-matomo-client-router.mjs:238:37
    at source.subscribe.isComplete (switchMap.js:14:23)
    at OperatorSubscriber._next (OperatorSubscriber.js:13:21)
    at OperatorSubscriber.next (Subscriber.js:31:18)
    at subscribe.innerComplete (mergeInternals.js:25:28)
    at OperatorSubscriber._next (OperatorSubscriber.js:13:21)

Following the stacktrace reveals that the trimTrailingSlash-function is called with a null value through the line:
trimTrailingSlash(this.baseHref ?? this.platformLocation.getBaseHrefFromDOM());
Therefore, it seems that in our situation, getBaseHrefFromDOM returns null as well. After looking at the documentation of PlatformLocation, it states: 'This class should not be used directly by an application developer. Instead, use Location.'.

Although I am unsure why getBaseHrefFromDOM() returns null, perhaps this is a side-effect of using a class that should not be called by a developer. Do you see a way to resolve this, by perhaps retrieving the value in a different way?

@EmmanuelRoux
Copy link
Owner

Hi @Viserius

Thanks for reporting this bug

You're right, LocationStrategy#getBaseHref should probably used instead.

(But in fact, LocationSatrategy is delegating to platformLocation.getBaseHrefFromDO... so I wonder why this bug appears. Could you provide a repro ?)

Feel free to open a PR to implement your suggested change, I will merge it soon !

@Viserius
Copy link
Contributor Author

Although my PR does not solve the issue, using a HashLocationStrategy as opposed to the default PathLocationStrategy might be the problem.

@EmmanuelRoux EmmanuelRoux added the bug Something isn't working label Jan 30, 2024
@Viserius
Copy link
Contributor Author

In our case, the null value might be caused by the fact that we do not specify a certain tag in our HTML, causing the value to be null, meaning nonexistent. I will propose a bugfix.

@EmmanuelRoux
Copy link
Owner

Ok, the problem seems to come from a bad typing in Angular source code. See here (L120-124) and here (L71-74)

  override getBaseHrefFromDOM(): string {
    return getDOM().getBaseHref(this._doc)!; // <-- This can actually be null !
  }
  override getBaseHref(doc: Document): string|null {
    const href = getBaseElementHref();
    return href == null ? null : relativePath(href);
  }

I will provide a workaround in addition to your PR, thanks for identifying this problem!

@Viserius
Copy link
Contributor Author

@EmmanuelRoux Thank you for the unbelievably fast response times and thinking with me. Looking forward to using a new version. Any idea when you're able to make a release with this fix?

@EmmanuelRoux
Copy link
Owner

@Viserius I've added some comments to your PR to suggest something, please take a look

EmmanuelRoux pushed a commit that referenced this issue Jan 30, 2024
EmmanuelRoux added a commit that referenced this issue Jan 30, 2024
## [6.0.2](v6.0.1...v6.0.2) (2024-01-30)

### Bug Fixes

* **router:** retrieve Base Href from `LocationStrategy` and correctly handle nulls ([73e8442](73e8442)), closes [#82](#82)
@EmmanuelRoux
Copy link
Owner

🎉 This issue has been resolved in version 6.0.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working released
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants