Breaking/tdr 9/handlebars 4 upgrade #1373
Conversation
|
Code Climate has analyzed commit dfdb4b8 and detected 0 issues on this pull request. View more on Code Climate. |
Codecov ReportPatch coverage has no change and project coverage change:
Additional details and impacted files@@ Coverage Diff @@
## develop #1373 +/- ##
=============================================
- Coverage 16.06% 16.06% -0.01%
- Complexity 4207 4208 +1
=============================================
Files 391 391
Lines 13491 13492 +1
=============================================
Hits 2168 2168
- Misses 11323 11324 +1 ☔ View full report in Codecov by Sentry. |
Version🚨 Your pull request contains a BREAKING CHANGE, please be sure to communicate it.
There are 2 BREAKING CHANGES, 2 features, 0 fix |
There was a problem hiding this comment.
This file is a copy of one of the handlebars 4.7.8 modules installed in node_modules. I was in 2 minds, but I placed it here in portableLib (beside handlebars 1.3.0) to match the way some current PCIs (not all) are resolving these "portable" modules. If this is ok, I could remove handlebars.js (unless you think we should hold onto it for backwards compatibility).
There was a problem hiding this comment.
I'm pretty sure this file is never even called by any of the PCIs I've tested.
There was a problem hiding this comment.
Indeed, I did not find any usage. However, some 3rd-party PCI providers could use it.
There was a problem hiding this comment.
The idea of using a different namespace for Handlebars in PCI is good. However, it will only work if we can make sure the lib is loaded before. This is the tricky part: we rely on the same mechanism to load the helpers both for the PCI compiler and the regular TAO compiler. The regular compiler will preload the system default Handlebars, we need it for TAO related bundles. When discovering a PCI we need to replace it by the portableLib, but at this stage it is already too late as the RequireJS dependencies tree is already built... If you have some idea, you are welcome :)
Look at https://oat-sa.atlassian.net/browse/ADF-1293 for having a use case.
| @@ -34,7 +34,7 @@ define([], function () { | |||
| return function moduleWriter(moduleName, compiled) { | |||
| let handlebars = 'handlebars'; | |||
| if (isPciRuntime(moduleName)) { | |||
| handlebars = 'taoQtiItem/portableLib/handlebars'; | |||
| handlebars = 'taoQtiItem/portableLib/handlebars_4'; | |||
There was a problem hiding this comment.
issue: The idea looks very good. However, we need to make sure that taoQtiItem/portableLib/handlebars_4 is loaded in the first place. Without this, the runtime will declare a dependency on an inexistent package...
|
@mike-ionut-mihai-sandu-tao just to remind you, that you still didn't pay my invoice. |
https://oat-sa.atlassian.net/browse/TR-3264
https://oat-sa.atlassian.net/wiki/spaces/FOUN/pages/144081031/Upgrade+Handlebars
This extension is a bit more complex because it contains the
portableLibfolder for PCIs, and theportablelementgrunt task definition.