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

Upgrade Mockup to version 4, patternslib to version 3 and jQuery to 3.5.1. #102

Closed
wants to merge 5 commits into from

Conversation

thet
Copy link
Member

@thet thet commented Oct 9, 2020

No description provided.

@mister-roboto
Copy link

@thet thanks for creating this Pull Request and help improve Plone!

To ensure that these changes do not break other parts of Plone, the Plone test suite matrix needs to pass.

Whenever you feel that the pull request is ready to be tested, either start all jenkins jobs pull requests by yourself, or simply add a comment in this pull request stating:

@jenkins-plone-org please run jobs

With this simple comment all the jobs will be started automatically.

Happy hacking!

@thet
Copy link
Member Author

thet commented Oct 9, 2020

@jenkins-plone-org please run jobs

@mauritsvanrees
Copy link
Member

Is this meant for Plone 5.2? I can't really tell whether that is wise and what it could possibly break.
Do site owners or add-on creators need to update anything?

I may want to do a Plone 5.2.3 release in one or two weeks, and this sounds possibly dangerous, if only because of the massive amounts of files changed. But maybe they are all tiny, safe changes.
(But Jenkins seems a bit too unstable the last few days to feel safe about doing any release soon.)

@thet
Copy link
Member Author

thet commented Oct 9, 2020

At least this should made available for Plone 5.x. I think it's even safe for Plone 5.2. It's basically an upgrade to jQuery 3 and patternslib 3. Addons which use really outdated API calls from jQuery might break, though.
See the changelog at: https://github.com/plone/mockup/tree/master/news
I did also a auto code formatting using prettier over the whole Mockup codebase.

@thet
Copy link
Member Author

thet commented Oct 9, 2020

I agree, 5000+ changed files are a lot...
This also includes all external dependencies of patternslib - which most of them we are not using.

@thet
Copy link
Member Author

thet commented Oct 9, 2020

Implication for:
Site owners: run the upgrade steps
Add On creators: None if JS code which is used isn't completely outdated.

@thet
Copy link
Member Author

thet commented Oct 12, 2020

@jenkins-plone-org please run jobs

@jensens
Copy link
Member

jensens commented Oct 12, 2020

I am also largely sceptical if this should go into 5.2.x ... I am fine to branch away plone.staticresources now and start a new major version with all the changes for Plone 6.

(I am pretty sure some of my sites will break).

@mauritsvanrees
Copy link
Member

We could talk about doing a 5.3. Larger changes such as this, and also bootstrap 4, might be okay there. Work on supporting Plone 5.2 would quickly stop then.
All code should then remain working on Python 2.7 and Archetypes as well. I don't know if that would mean more work for some packages.
Note that I don't think we need to update any Archetypes code to use bootstrap classes (although if someone wants to do the work, it should be fine.)
We can start a discussion on community.plone.org.

@jensens
Copy link
Member

jensens commented Oct 12, 2020

I dont think bootstrap 4 should go there either, this is a bigger change. I would really like to see 5.x frozen feature wise and only getting bugfixes. I am sure it would upset a whole bunch of people if we now start doing a 5.3 or not supporting 5.2.x any more. Lets focus on 6.0 and keep 5.2 as the stable beast with Python 2.7 support.

@thet
Copy link
Member Author

thet commented Oct 12, 2020

@jensens @mauritsvanrees this is not about Bootstrap 4
It's about "Mockup 4" which includes jQuery 3 and Patternslib 3.
The Mockup 4 version number should reflect that it has some bigger changes like jQuery 3.
jQuery 3 could break some stuff with JavaScript code from the last decade, so I'm fine to not include it in 5.2.

@thet
Copy link
Member Author

thet commented Oct 12, 2020

@jensens - the changes in "Mockup 4" (jQuery 3) should go into Plone 5.x IMO. Otherwise they won't land anywhere.
For Plone 6.x I want to incldue "Mockup 5", which uses ES6 JavaScript features, where there is already a working prototype: plone/mockup#1025
I think I should wrap this plan up in a PLIP.

@agitator
Copy link
Member

Barceloneta with Bootstrap 4 targets Plone 6, I won't invest any time for Plone 5.2 and absolutely not for Python 2.
If the Jquery update would work without much hassle on 5.2 then it would be fine with me but probably not a 5.3.
How about just releasing a mockup 4 and not using it as a default (in versions.cfg), similar as we did in the async resource loading profile - any adventures dev team could see if it works for them?
I'd also rather encourage people to upgrade to Plone 6 than to give any hope that a Plone 5.2 would be much more improved that bugfixes.

@jensens
Copy link
Member

jensens commented Oct 12, 2020

@thet I would support a plone.staticresources release targeting Plone 6 with the changes that also works in Plone 5.2 (but never goes into its release) for those lucky ones who want to try it. I understood, 5.2 is in maintenance mode.
But yes, a PLIP is the way to go.

@thet
Copy link
Member Author

thet commented Oct 12, 2020

@jensens @agitator the plone.staticresources branch which I plan for Plone 6 will not be compatible with 5.2 because it will not make use of RequireJS and almost no resource registry.
At least at the Plone 6 UI team I was talking a lot about this while laying out the plan for a restructured resource registry.
This PR is mainly meant as a last maintenance release to not forcing people to depend on a long outdated jQuery library for the next decade, while 5.2 is in maintenance mode. (jQuery 1.12.4 which we are currently using was released 4 years ago).
If this doesn't go in 5.2 I'd strongly recommend to release a 5.3.

Later today I will look into the reason why there are 5000+ file changes. Probably it's due to the updated Patternslib library which uses many external libraries (masonry, tippy, photoswipe, slick, anythingslider, etc) which we are not using but have to install due to the dependency chain.

The Jenkins failures are due to the usage of jQuery.size() which was deprecated with jQuery 1.8, 8 years ago. A fix is provided here: plone/Products.CMFPlone#3195 + plone/Products.CMFPlone#3196

@MrTango what's your opinion here? We were discussing this issue since some time now...

@thet
Copy link
Member Author

thet commented Oct 12, 2020

@jenkins-plone-org please run jobs

@mauritsvanrees
Copy link
Member

Regardless of in which version this ends up: a jQuery update is long overdue, so thanks for working on that!

@jensens
Copy link
Member

jensens commented Oct 13, 2020

I agree with @mauritsvanrees - this is awesome work and long overdue - thanks so much! Also, I think its save to merge in master anyway. Question is, to branch or not to branch from a point before the merge? And this is a question for both, the release and framework teams IMO.

@thet
Copy link
Member Author

thet commented Oct 13, 2020

@mauritsvanrees @jensens tnx!

As of the test failures I see that there are other places where the deprecated jQuery.size() is being used.
That makes it obvious that this IS a breaking change and should not go in a 5.2 release.
Later I will fix those jQuery.size() issues (needed for Plone 6 anyways and do no harm for 5.2) and will do a branch for a Mockup 3.x.

@MrTango
Copy link
Contributor

MrTango commented Oct 13, 2020

If the existing problems in core are fixed, it might be not a bad idea to go for a 5.3 with this important Jq-update.
Since the 5.2/3 will be there for quite a while, as it is the last version supping Python2. Having a decent Jquery version in there will help people who still relaying on Python2 for a while.

If we don't go for a 5.3, it would help if we have a working set of package version with this fixes, even if they don't belong to a Plone release. Who ever needs these fixes, can pin these version and use the newer versions in 5.2.

So branching befor merging makes sence anyway.

@jensens
Copy link
Member

jensens commented Oct 13, 2020

If we don't go for a 5.3, it would help if we have a working set of package version with this fixes, even if they don't belong to a Plone release. Who ever needs these fixes, can pin these version and use the newer versions in 5.2.

I think this is a good solution.

@mauritsvanrees
Copy link
Member

So then we would keep things stable and compatible by default, but people who know what they are doing can install newer versions of mockup and plone.staticresources, just like there was the option in Plone 4 to use plone.app.widgets. Sounds like a plan.

@thet Does that work for you too?

That does sound like there will be more work keeping branches working, back porting/forward porting.
And maybe extra testing on Travis or an extra job on Jenkins.

@thet
Copy link
Member Author

thet commented Oct 13, 2020

@jensens @mauritsvanrees @MrTango sounds good to me!

@thet
Copy link
Member Author

thet commented Oct 20, 2020

@jenkins-plone-org please run jobs

@ale-rt
Copy link
Member

ale-rt commented Nov 10, 2020

So then we would keep things stable and compatible by default, but people who know what they are doing can install newer versions of mockup and plone.staticresources, just like there was the option in Plone 4 to use plone.app.widgets. Sounds like a plan.

The @plone/framework-team is happy with this plan https://community.plone.org/t/fwt-meeting-minutes-2020-11-10/13086

@thet
Copy link
Member Author

thet commented Apr 30, 2021

I think we can close this. All effort is going into the es6 branch.
Let's re-visit if upgrade to jQuery 3 should be done for Plone 5.2.

@thet thet closed this Apr 30, 2021
@thet thet deleted the thet-update branch April 30, 2021 07:41
@mauritsvanrees
Copy link
Member

Half a year ago, or with more people working on it, the situation may have been different. But now it makes sense to focus on Plone 6.

@thet thet restored the thet-update branch May 6, 2021 13:05
@thet
Copy link
Member Author

thet commented May 6, 2021

People ask for an updated jQuery on Plone 5.2.
I restored the branch.

@petschki petschki deleted the thet-update branch April 17, 2023 12:59
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.

7 participants