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.

Sign up for GitHub

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

Jump to bottom

Faces.js and Maven compressor #5124

Closed
wants to merge 1 commit into from
Closed

Faces.js and Maven compressor #5124

wants to merge 1 commit into from

Conversation

pizzi80
Copy link
Contributor

@pizzi80 pizzi80 commented Jun 20, 2022

As promised a long time ago, this is my contribution to Mojarra:
This is a first release which can be used as a base for further code cleanups and optimizations.

Maven

  1. Removed yuicompressor (dead project with no support for ES6+ js)
  2. Added Google Closure maven plugin for minification
    https://github.com/blutorange/closure-compiler-maven-plugin

Faces JS

  1. tested and minified with ECMASCRIPT5_STRICT profile of Google Closure
  2. use strict
  3. const and let instead of var
  4. Removed IE-5-6-7-8-9-10 support
  5. Removed IE 5-6-7 hacks and memory hacks
  6. Removed FrameTransport
  7. FormData for File upload
  8. Many minor optimizations
  9. Removed / Commented out unused and very old functions
  10. ~ 10 kB Minified and Gzipped

#5045
#5040

sorry if I made some mistake with PR or similar... I'm not used to contribute to open source github projects

@pizzi80
Maven:
7e92c2c 
 - removed yuicompressor (dead project)
 + added closure compiler

faces-uncompressed.js:
 - build with ECMASCRIPT5_STRICT profile of Google Closure
 - use strict
 - Removed IE-5-6-7-8-9-10 support
 - Removed FrameTransport
 - Use FormData for File upload
 - removed IE hacks and memory hacks
 - const and let instead of var
 - many minor optimization
@melloware
Copy link
Contributor

This looks great but it looks like you need to sign the legal agreement and then watch this video how to fix this PR with your signature! https://www.youtube.com/watch?v=YdJyJpT7rrs

@pizzi80
Copy link
Contributor Author

pizzi80 commented Jun 20, 2022

jakartaee/faces#1598

@pizzi80
Copy link
Contributor Author

pizzi80 commented Jun 20, 2022

This looks great but it looks like you need to sign the legal agreement and then watch this video how to fix this PR with your signature! https://www.youtube.com/watch?v=YdJyJpT7rrs

I've created an Eclispe account wired with this github account ... I need to follow the instructions on the video... ;)

melloware
melloware suggested changes Jun 20, 2022
View reviewed changes
impl/src/main/resources/META-INF/resources/jakarta.faces/faces-uncompressed.js
// if( isIE() && (source.getAttribute(propertyName) === source[propertyName]) ){
// attributeName = propertyName;
// }
// }
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like extra comment here can be removed?

impl/src/main/resources/META-INF/resources/jakarta.faces/faces-uncompressed.js
// target[propertyName] = '';
// }
// target.removeAttribute(attributeName);
// }
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here?

impl/src/main/resources/META-INF/resources/jakarta.faces/faces-uncompressed.js
// doc.loadXML(docStr);
// } else {
// throw new Error("You don't seem to be running a supported browser");
// }
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove commented code.

impl/src/main/resources/META-INF/resources/jakarta.faces/faces-uncompressed.js
// if (formElement.name && (formElement.name.indexOf('jakarta.faces.encodedURL') >= 0)) {
// return formElement;
// }
// }
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove commented code.

impl/src/main/resources/META-INF/resources/jakarta.faces/faces-uncompressed.js
// if (formElement.name && (formElement.name.indexOf(hiddenStateFieldName) >= 0)) {
// return formElement;
// }
// }
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove commented code

impl/src/main/resources/META-INF/resources/jakarta.faces/faces-uncompressed.js
// isInTable = true;
// break;
// }
// }
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove commented code.

@pizzi80
Copy link
Contributor Author

pizzi80 commented Jun 20, 2022

@melloware I was not sure if it was better to remove everything or not... it's a 3kLOC single file full of hacks..... now that you told me to remove everything, tomorrow I will continue to work on it 😉😉

@melloware
Copy link
Contributor

To me I hate leaving dead blocks of code. That is what Git history is for!

@pizzi80 pizzi80 closed this Jun 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@melloware melloware melloware requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@pizzi80 @melloware