LPS-124465 Avoid unwanted minification of Liferay global
#636
Conversation
As explained in https://issues.liferay.com/browse/LPS-124465 our minification takes some care to avoid mangling property access: https://github.com/liferay/liferay-frontend-projects/blob/a33874a2630a07e24cd46421fb346235467ea6c5/projects/npm-tools/packages/npm-scripts/src/config/terser.config.js#L10-L12 Combined with the fact that `Liferay` is usually accessed as a global and therefore not minified, that means that we can rely on the minifier leaving strings like `Liferay.Language.get` untouched in the source. The LanguageFilter can then find those strings and substitute them on the fly. In LPS-124465 we see a pattern that breaks this. `Liferay` is passed into a function, where, as a local binding, the minifier is free to rewrite it (eg. to `t`), so our source text now reads `t.Language.get` and the LanguageFilters does not substitute it. Fix taken here is to avoid that pattern in `portlet.js` and everywhere else we seem to be doing it (not many places, all found with a crude regex for `\((\s*\w+\s*,)*\bLiferay\b(\s*,\s*\w+\s*)*\)`; that won't cover everything, like things split across multiple lines, but it will catch most things... probably everything). Instead of: ```js (function (Liferay) { // blah... })(Liferay); ``` we just do: ```js (function () { // blah... })(); ``` There was one place where we had: ```js Liferay = window.Liferay || {}; ``` Note that the minifier is "smart" enough to know that `Liferay` there is not a global, and it preserves the name. But prior to this PR, if you inspect the build output it does the unwanted `t` mangling; ie. before: Liferay=window.Liferay||{},function(t){ /* a bunch of stuff */ }(Liferay); after: Liferay=window.Liferay||{},function(){ /* a bunch of stuff */ }();
|
CI is automatically triggering the following test suites:
|
|
Now observe SF blow up on this one because I tried to run it locally and it ran out of memory (probably due to having an |
|
ci:test:bundle |
|
We could look at adding a lint to stop anybody from doing anything like this again, but I am lazy, and propose that we only do that if it actually does happen again. |
liferay-continuous-integration
added
ci:test:sf - success
and removed
ci:test:sf - pending
labels
✔️ ci:test:sf - 1 out of 1 jobs passed in 0 msClick here for more details.Base Branch:Branch Name: master Sender Branch:Branch Name: wincent/LPS-124465/language-get 1 Successful Jobs:For more details click here. |
Or if somebody paid us 5 Lifebucks. |
|
Jenkins Build:test-portal-source-format#4760 |
liferay-continuous-integration
added
ci:test:bundle - success
and removed
ci:test:bundle - pending
labels
✔️ ci:test:bundle - 1 out of 1 jobs passed in 53 minutesClick here for more details.Base Branch:Branch Name: master Copied in Private Modules Branch:Branch Name: master-private ci:test:bundle - 1 out of 1 jobs PASSEDFor more details click here.Test bundle downloads:
|
|
Regression is gone:
|
This answers #629 (comment), so I'd say let's get on with
|
|
Forwarded manually to brianchandotcom#97045 |
|
Jenkins Build:test-portal-acceptance-pullrequest(master)#4919 |
And here too: #632 |
|
Jenkins Build:test-portal-acceptance-pullrequest(master)#4919 |
As explained in https://issues.liferay.com/browse/LPS-124465 our minification takes some care to avoid mangling property access.
Combined with the fact that
Liferayis usually accessed as a global and therefore not minified, that means that we can rely on the minifier leaving strings likeLiferay.Language.getuntouched in the source. The LanguageFilter can then find those strings and substitute them on the fly.In LPS-124465 we see a pattern that breaks this.
Liferayis passed into a function, where, as a local binding, the minifier is free to rewrite it (eg. tot), so our source text now readst.Language.getand the LanguageFilter does not substitute it.Fix taken here is to avoid that pattern in
portlet.jsand everywhere else we seem to be doing it (not many places, all found with a crude regex for\((\s*\w+\s*,)*\bLiferay\b(\s*,\s*\w+\s*)*\); that won't cover everything, like things split across multiple lines, but it will catch most things... probably everything).Instead of:
we just do:
There was one place where we had:
Note that the minifier is "smart" enough to know that
Liferaythere is not a global, and it preserves the name. But prior to this PR, if you inspect the build output it does the unwantedtmangling; ie. before:after: