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

LPS-124871 Propagate NODE_ENV to YUI loader #632

Conversation

wincent
Copy link

@wincent wincent commented Dec 17, 2020

Our other resources are toggled between minified and non-minified form based on the value of NODE_ENV, so we do the same here in the name of consistency for resources that are loaded via the YUI loader.

This means we effectively ignore the old logic that consults themeDisplay.isThemeJsFastLoad() and the javascript.log.enabled property, in order to return 'raw', 'min' or 'debug' as a filter type. Now, we only ever return 'raw' or 'min'.

If you're curious, how these filter types are used can be seen in the loader source.

Our other resources are toggled between minified and non-minified form
based on the value of `NODE_ENV`, so we do the same here in the name of
consistency for resources that are loaded via the YUI loader.

This means we effectively ignore the old logic:

https://github.com/liferay/liferay-portal/blob/012adf4b2836179a5478bf04b5337955917fccb8/portal-web/docroot/html/common/themes/top_js.jspf#L329-L342

that consults `themeDisplay.isThemeJsFastLoad()` and the
`javascript.log.enabled` property, in order to return 'raw', 'min' or
'debug' as a filter type. Now, we only ever return 'raw' or 'min'.

If you're curious, how these filter types are used can be seen in the
loader source:

https://alloyui.com/api/files/yui3_src_loader_js_loader.js.html
@liferay-continuous-integration
Copy link
Collaborator

CI is automatically triggering the following test suites:

  •     ci:test:relevant
  •     ci:test:sf

@wincent
Copy link
Author

wincent commented Dec 17, 2020

I'm still testing this manually, but sending this here early so that I can give CI a head-start on it. I'll report back with my findings.

@liferay-continuous-integration
Copy link
Collaborator

✔️ ci:test:sf - 1 out of 1 jobs passed in 4 minutes

Click here for more details.

Base Branch:

Branch Name: master
Branch GIT ID: ff0e5e147ea14773fc626b3a4024d041bfefb138

Sender Branch:

Branch Name: wincent/LPS-124871/aui-minification
Branch GIT ID: c4ef15e3734ab1db3ae574bba6336cb3d47a9567

1 out of 1jobs PASSED
1 Successful Jobs:
For more details click here.

Copy link

@jbalsas jbalsas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Keanu approves!

kean_approved

@liferay-continuous-integration
Copy link
Collaborator

@wincent
Copy link
Author

wincent commented Dec 17, 2020

Well, you know, that's kind of disappointing. I thought this one went through Babel (it does) but forgot that it doesn't go through the bundler, which is the one responsible for setting up the plug-ins to transform. At least, that's IIRC.

Screenshot 2020-12-17 -151520-uP1Yr1AL@2x

Going to see if I can wire up Babel as required.

@wincent
Copy link
Author

wincent commented Dec 17, 2020

ci:stop

@liferay-continuous-integration
Copy link
Collaborator

❌ ci:test:relevant - 0 out of 1 jobs passed in 52 minutes

Click here for more details.

Base Branch:

Branch Name: master
Branch GIT ID: ff0e5e147ea14773fc626b3a4024d041bfefb138

Copied in Private Modules Branch:

Branch Name: master-private
Branch GIT ID: 0ff04d9f73a4643abdd2b335581feef26b985b8a

ci:test:relevant - 0 out of 1 jobs PASSED
For more details click here.

Failures unique to this pull:

  1. test-portal-acceptance-pullrequest(master)
    Job Results:

    0 Jobs Passed.
    1 Job Failed.

    Build was aborted

For upstream results, click here.

@jbalsas
Copy link

jbalsas commented Dec 17, 2020

Well, you know, that's kind of disappointing. I thought this one went through Babel (it does) but forgot that it doesn't go through the bundler, which is the one responsible for setting up the plug-ins to transform. At least, that's IIRC.

Huh... I would've fell for the same

Fixes:

    modules.js:52 Uncaught ReferenceError: process is not defined

caused by:

    filter: process.env.NODE_ENV === 'development' ? 'raw' : 'min'

not getting appropriately transformed in the source. This is because,
while the code _does_ go through Babel (thanks to npm-scripts), this old
AUI code does _not_ go through the Bundler (due to the `.npmbundlerrc`
config), and it's the bundler which is responsible for this particular
transform (we may want to reconsider that though... especially as we
move away from v2 to v3).

Test plan: Inspect built source and see no `process` reference; note
that the filter code has been minified and inlined to:

    !function(){var e=Liferay.AUI/* etc.. */,filter:"min",
@wincent
Copy link
Author

wincent commented Dec 17, 2020

Fixed in b9db3c1 and made a note in there that we might want to move this particular plugin (babel-plugin-transform-node-env-inline) into our npm-scripts-managed preset and out of the bundler, just to be safe... thoughts?

@wincent
Copy link
Author

wincent commented Dec 17, 2020

ci:test:sf

@wincent
Copy link
Author

wincent commented Dec 17, 2020

ci:test:relevant

@wincent
Copy link
Author

wincent commented Dec 17, 2020

ci:test:bundle

@liferay-continuous-integration
Copy link
Collaborator

✔️ ci:test:sf - 1 out of 1 jobs passed in 5 minutes

Click here for more details.

Base Branch:

Branch Name: master
Branch GIT ID: ff0e5e147ea14773fc626b3a4024d041bfefb138

Sender Branch:

Branch Name: wincent/LPS-124871/aui-minification
Branch GIT ID: b9db3c1b71a7b3d36534a3806e9ac0ac539a58f7

1 out of 1jobs PASSED
1 Successful Jobs:
For more details click here.

@liferay-continuous-integration
Copy link
Collaborator

@wincent
Copy link
Author

wincent commented Dec 17, 2020

So with this applied I see -min.js files getting loaded that look like they came from the AUI loader, so I guess this works?

Screenshot 2020-12-17 -154003-nCnkGKqH@2x

@liferay-continuous-integration
Copy link
Collaborator

@liferay-continuous-integration
Copy link
Collaborator

✔️ ci:test:bundle - 1 out of 1 jobs passed in 1 hour 3 minutes

Click here for more details.

Base Branch:

Branch Name: master
Branch GIT ID: ff0e5e147ea14773fc626b3a4024d041bfefb138

Copied in Private Modules Branch:

Branch Name: master-private
Branch GIT ID: 0ff04d9f73a4643abdd2b335581feef26b985b8a

ci:test:bundle - 1 out of 1 jobs PASSED
For more details click here.
Test bundle downloads:

@liferay-continuous-integration
Copy link
Collaborator

✔️ ci:test:stable - 9 out of 9 jobs passed

❌ ci:test:relevant - 20 out of 23 jobs passed in 2 hours 59 minutes

Click here for more details.

Base Branch:

Branch Name: master
Branch GIT ID: ff0e5e147ea14773fc626b3a4024d041bfefb138

Copied in Private Modules Branch:

Branch Name: master-private
Branch GIT ID: 0ff04d9f73a4643abdd2b335581feef26b985b8a

ci:test:stable - 9 out of 9 jobs PASSED
9 Successful Jobs:
ci:test:relevant - 20 out of 23 jobs PASSED
20 Successful Jobs:
For more details click here.

Failures unique to this pull:


Failures in common with acceptance upstream results at ff0e5e1:
  1. test-portal-acceptance-pullrequest-batch(master)/semantic-versioning-jdk8/0
    Job Results:

    0 Tests Passed.
    1 Test Failed.

    1. AXIS_VARIABLE=0,label_exp=!master #457421

      Please fix semantic versioning on wincent/wincent/LPS-124871/aui-minification

           [exec] 			+   return     void
           [exec] 		+   method     <init>(java.lang.String)
           [exec] 			+   return     void
           [exec] 		+   method     <init>(java.lang.String,java.lang.Throwable)
           [exec] 			+   return     void
           [exec] 		+   method     <init>(java.lang.Throwable)
           [exec] 			+   return     void
           [exec] 		+   method     addSuppressed(java.lang.Throwable)
           [exec] 			+   access     final
           [exec] 			+   return     void
           [exec] 		+   method     clone()
           [exec] 			+   access     protected
           [exec] 			+   return     java.lang.Object
           [exec] 		+   method     equals(java.lang.Object)
           [exec] 			+   return     boolean
           [exec] 		+   method     fillInStackTrace()
           [exec] 			+   return     java.lang.Throwable
           [exec] 		+   method     finalize()
           [exec] 			+   access     protected
           [exec] 			+   return     void
           [exec] 		+   method     getCause()
           [exec] 			+   return     java.lang.Throwable
           [exec] 		+   method     getLocalizedMessage()
           [exec] 			+   return     java.lang.String
           [exec] 		+   method     getMessage()
           [exec] 			+   return     java.lang.String
           [exec] 		+   method     getStackTrace()
           [exec] 			+   return     java.lang.StackTraceElement[]
           [exec] 		+   method     getSuppressed()
           [exec] 			+   access     final
           [exec] 			+   return     java.lang.Throwable[]
           [exec] 		+   method     hashCode()
           [exec] 			+   return     int
           [exec] 		+   method     initCause(java.lang.Throwable)
           [exec] 			+   return     java.lang.Throwable
           [exec] 		+   method     printStackTrace()
           [exec] 			+   return     void
           [exec] 		+   method     printStackTrace(java.io.PrintStream)
           [exec] 			+   return     void
           [exec] 		+   method     printStackTrace(java.io.PrintWriter)
           [exec] 			+   return     void
           [exec] 		+   method     setStackTrace(java.lang.StackTraceElement[])
           [exec] 			+   return     void
           [exec] 		+   method     toString()
           [exec] 			+   return     java.lang.String
           [exec] [Baseline Report] Mode: diff (persisted)
           [exec] Semantic versioning is incorrect while checking /opt/dev/projects/github/liferay-portal/portal-kernel/portal-kernel.jar against /opt/dev/projects/github/liferay-portal/.gradle/caches/modules-2/files-2.1/com.liferay.portal/com.liferay.portal.kernel/9.18.0/471e51c2d8cc477362ba9dd7449b8c3974545660/com.liferay.portal.kernel-9.18.0.jar

@liferay-continuous-integration
Copy link
Collaborator

@wincent
Copy link
Author

wincent commented Dec 17, 2020

Failure is spurious (see: #636) so going to manual forward.

@wincent
Copy link
Author

wincent commented Dec 17, 2020

Manual forward: brianchandotcom#97050

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants