@diosmosis opened this Pull Request on October 17th 2021 Member

Description:

Changes:

  • add static AjaxHelper.fetch method to make it seem more like piwikApi service
  • migrate RateFeature component
  • modify MatomoDialog to use v-model so it's simpler to use from vue
  • made vue: property optional in scope mapping in createAngularAdapter
  • migrate enrichedheadline
  • allow translate function to take variadic args OR array of strings like _pk_translate
  • migrate contentblock component
  • (not related to vue) fix anchor links in trackingcodegenerator page (linking to #anchor instead of #/anchor causes angularjs to fail, happens on 4.x-dev too)
  • fix nested transclude issue w/ createAngularAdapter

Notes:

image

image

image

Note: it seems adding components does bump the the JS payload a bit. The 4 new vue component templates add about 2kb to the gzipped content. It might be good to set a threshold for when it would be good to explore either using async components or loading plugin UMD's lazily. Both are doable, but it would be easier to manage the latter option in the long run.

Review

@sgiehl commented on October 25th 2021 Member

@diosmosis Unrelated to this PR, I get the following notice in the browser console

Source-Map-Fehler: Error: NetworkError when attempting to fetch resource.
Ressourcen-Adresse: webpack:////home/dizzy/Projects/matomo/node_modules/dompurify/dist/purify.js?

Seems the path is your local one...

@diosmosis commented on October 25th 2021 Member

@sgiehl I grepped for it and its in the source map for the polyfill build... not sure why it's using absolute paths, the build config might be wrong there... I'll take a look, thanks for mentioning it!

@diosmosis commented on October 25th 2021 Member

@sgiehl should be fixed

@diosmosis commented on October 25th 2021 Member

Actually it seems the recommended approach is to not include the sourcemap in production: https://webpack.js.org/configuration/devtool/, but since we distribute unminified javascript with plugins (and much of Matomo is open source), perhaps this isn't something we need to care about... cc @sgiehl @tsteur

@diosmosis commented on October 25th 2021 Member

Actually, actually, since we just merge everything into one big JavaScript asset, the sourcemaps are useless in production.

@diosmosis commented on October 26th 2021 Member

@sgiehl some test failures but they don't seem related

@sgiehl commented on October 26th 2021 Member

Actually, actually, since we just merge everything into one big JavaScript asset, the sourcemaps are useless in production.

Should we remove them from the build then, so they aren't included in the release zip?

@sgiehl commented on October 26th 2021 Member

Tests seem to pass. Will merge this now.

@diosmosis commented on October 26th 2021 Member

@sgiehl

Should we remove them from the build then, so they aren't included in the release zip?

I guess we could, they're only really useful for development, and they'll be generated when we develop. I'll make a PR.

This Pull Request was closed on October 26th 2021
Powered by GitHub Issue Mirror