? Pending

User tests: Successful: Unsuccessful:

avatar wojsmol
wojsmol
2 Aug 2018

Pull Request for Issue # .

Summary of Changes

This PR reverts build/build-modules-js/compile-es6.js to https://raw.githubusercontent.com/joomla/joomla-cms/0d996e30749bbeee4681b02a5e1be0dd91addb2e/build/build-modules-js/compile-es6.js removing testing changes made in PR #21350

Problem is reproducible at least on Windows.

Testing Instructions

code review
or install 4.0-dev on this commit from github - see docs

after applying this you must run npm run build:js

Expected result

no_admin_erroradministration

Actual result

admin_error

Documentation Changes Required

no

cc @wilsonge @dgrammatiko

avatar wojsmol wojsmol - open - 2 Aug 2018
avatar wojsmol wojsmol - change - 2 Aug 2018
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 2 Aug 2018
Category JavaScript Repository
avatar wojsmol wojsmol - change - 2 Aug 2018
The description was changed
avatar wojsmol wojsmol - edited - 2 Aug 2018
avatar wojsmol wojsmol - change - 2 Aug 2018
The description was changed
avatar wojsmol wojsmol - edited - 2 Aug 2018
avatar wojsmol wojsmol - change - 2 Aug 2018
The description was changed
avatar wojsmol wojsmol - edited - 2 Aug 2018
avatar wojsmol wojsmol - change - 3 Aug 2018
Title
fixd es6 compilation
fix es6 compilation
avatar wojsmol wojsmol - edited - 3 Aug 2018
avatar wojsmol wojsmol - change - 3 Aug 2018
The description was changed
avatar wojsmol wojsmol - edited - 3 Aug 2018
avatar roland-d roland-d - test_item - 3 Aug 2018 - Tested successfully
avatar roland-d
roland-d - comment - 3 Aug 2018

I have tested this item successfully on 0a7b18c

I was unable to login to the admin screen, after applying the changes and run npm install. After NPM was finished, I saw the login screen.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/21371.

avatar wojsmol
wojsmol - comment - 3 Aug 2018

@roland-d npm run build:js is match master but results are the same ?

avatar dgrammatiko
dgrammatiko - comment - 3 Aug 2018

@wojsmol this code is really redundant...

avatar roland-d
roland-d - comment - 3 Aug 2018

I don't know, I am told to run npm install and I can't login anymore.....

avatar dgrammatiko
dgrammatiko - comment - 3 Aug 2018

@roland-d what are the errors in the console.log of your browser?
Were there any errors when running npm install

avatar roland-d
roland-d - comment - 3 Aug 2018

@dgrammatiko There are always issues when I run npm install :)

This is on a Windows laptop of one of the students:

npm WARN ajv-keywords@3.2.0 requires a peer of ajv@^6.0.0 but none is installed. You must install peer dependencies yourself.
npm WARN babel-loader@6.4.1 requires a peer of webpack@1 || 2 || ^2.1.0-beta || ^2.2.0-rc but none is installed. You must install peer dependencies yourself.
npm WARN sass-loader@5.0.1 requires a peer of webpack@^2.0.0 but none is installed. You must install peer dependencies yourself.
npm WARN com_media@4.0.0 No repository field.

SKIPPING OPTIONAL DEPENDENCY: fsevents@1.2.4 (node_modules\fsevents)
SKIPPING OPTIONAL DEPENDENCY: Unsupported platform for fsevents@1.2.4: wanted {"os":"darwin","arch":"any"} {"os":"win32","arch":"x64"} 

On my Mac I get this:

npm WARN ajv-keywords@3.2.0 requires a peer of ajv@^6.0.0 but none is installed. You must install peer dependencies yourself.
npm WARN babel-loader@6.4.1 requires a peer of webpack@1 || 2 || ^2.1.0-beta || ^2.2.0-rc but none is installed. You must install peer dependencies yourself.
npm WARN sass-loader@5.0.1 requires a peer of webpack@^2.0.0 but none is installed. You must install peer dependencies yourself.
npm WARN com_media@4.0.0 No repository field.

audited 8385 packages in 67.258s
found 20 vulnerabilities (10 low, 9 moderate, 1 high)
avatar dgrammatiko
dgrammatiko - comment - 3 Aug 2018

These messages are ok, on the windows os some packages are not avilable (needed) The peer dependencies are coming from the media manager and are also ok.
The thing with this piece of code is:

  • it's wrong (will search and tranpile all the es6.js files in place) wich will ruin our source folders!!!
  • it was ment to be called directly, but we don't want to directly call this module, we have already 2 commands: npm run build:js and node build.js --compile-js
  • The command node build.js --compile-js will also accept a specific source folder so someone theretically can compile only the files teir working on, although we have a watch function fo this
avatar roland-d
roland-d - comment - 3 Aug 2018

@dgrammatiko The code may be wrong but if users don't get a login page, we search for solutions :)

avatar dgrammatiko
dgrammatiko - comment - 3 Aug 2018

Ok, can you please provide me the js files that the browser is missing without this PR?
npm install here on the login page I have:
screenshot 2018-08-03 at 13 39 55

Also what is the OS you're running ?

avatar roland-d
roland-d - comment - 3 Aug 2018

@dgrammatiko I have tried to get the issue again but no matter what I try, I always get the login screen this time.

The OS is Windows 10.0.17134.165.

avatar dgrammatiko
dgrammatiko - comment - 3 Aug 2018

@wilsonge this PR is wrong please don't merge

avatar wojsmol wojsmol - change - 3 Aug 2018
Status Pending Closed
Closed_Date 0000-00-00 00:00:00 2018-08-03 14:41:07
Closed_By wojsmol
Labels Added: ?
avatar wojsmol wojsmol - close - 3 Aug 2018
avatar wojsmol
wojsmol - comment - 3 Aug 2018

@dgrammatiko PR closed

avatar bene-we
bene-we - comment - 6 Aug 2018

I also cannot login, these are the javascript files that are loaded:

- passwordview.min.js
- validate.min.js
- core.min.js
- keepalive.min.js
- focus-visible.min.js
- joomla-alert.min.js
- punycode.min.js

Should I install this PR and test if it's working afterwards?
@roland-d

avatar wojsmol
wojsmol - comment - 6 Aug 2018

@ggppdk Competing list from @bene-we with your screenshot missing script is admin-login.min.js.

avatar ggppdk
ggppdk - comment - 6 Aug 2018

@wojsmol did you mean to reference me ?
do you ask me to test something ?

avatar wojsmol
wojsmol - comment - 6 Aug 2018

@ggppdk no sorry ?
@dgrammatiko See comment from @bene-we.

avatar dgrammatiko
dgrammatiko - comment - 6 Aug 2018

@wojsmol I need some information here so I can replicate the issue here. So please provide:
OS ver
LAMP (eg xamp or...) version

avatar wojsmol
wojsmol - comment - 6 Aug 2018

@dgrammatiko In my case:
Windows 8.1
LAMP jamp 3.0.8(trial version downloadable from here - 14 days trial)

avatar bene-we
bene-we - comment - 6 Aug 2018

I'm running Windows 10.0.17134 Build 17134 Pro,
xampp version is 3.2.2

The error ocurred in Chrome and Edge

avatar dgrammatiko
dgrammatiko - comment - 6 Aug 2018

@wojsmol @bene-we what happens if you run the npm install twice? Do you get all the files for the admin login?

avatar roland-d
roland-d - comment - 6 Aug 2018

@dgrammatiko Running npm install for a second time fixes it. Why?

avatar dgrammatiko
dgrammatiko - comment - 6 Aug 2018

@roland-d the script is promise based, maybe something is running early and thus not doing what is expected

PS Now debugging an npm cli is going to be fun... fml

avatar wojsmol
wojsmol - comment - 6 Aug 2018

@dgrammatiko In my case when errors occurred It was reproducible on 2 computers - my and @zwiastunsw.

avatar roland-d
roland-d - comment - 6 Aug 2018

So we just have to pray every time we do an update of the git repo that this works?

avatar mbabker
mbabker - comment - 6 Aug 2018

So we just have to pray every time we do an update of the git repo that this works?

Or stop using Windoze

/me hobbles for cover

avatar brianteeman
brianteeman - comment - 6 Aug 2018

i had better results running npm in WSL on my Windows 10 box but always one script at a time and NOT all in the one build

avatar dgrammatiko
dgrammatiko - comment - 6 Aug 2018

Well, I've spotted a couple possible bugs by checking the code. I need to setup a VM and check all these stuff myself. Probably later on (not in a mood to install windows right now)

avatar bene-we
bene-we - comment - 6 Aug 2018

Running npm install_ again took nearly 10 minutes, but after that the script is loaded and the login screen works as expected.
I recognized this line in the terminal window:

Compiling: C:\xampp\htdocs\joomla-cms\build\media_src\mod_login\js\admin-login.js

This line also appeared with some other *.js files, and before that there were a huge amount of .js files listed and it said Processing:

avatar dgrammatiko
dgrammatiko - comment - 6 Aug 2018

@wojsmol @bene-we @ggppdk I think #21427 should fix the windows problem

Add a Comment

Login with GitHub to post a comment