Request for feedback on React Boilerplate for Jenkins Plugin

31 views
Skip to first unread message

Jack Shen

unread,
Aug 22, 2019, 5:43:42 AM8/22/19
to Jenkins Developers
Hi,
    I had set up a boilerplate repository about using React Boilerplate to develop Jenkins Plugin, which is part of the targets of GSoC 2019 Project Working Hours Plugin UI Improvement.
    I hope this boilerplate could help Jenkins developers' way to build plugins much easier.
    The repository could be found at https://github.com/ShenJack/react-boilerplate-for-jenkins-plugin, for more detail about this boilerplate, you can read the README.md
    Thanks for reading, and any feedback are truly appreciated.

Oleg Nenashev

unread,
Aug 22, 2019, 11:48:35 AM8/22/19
to Jenkins Developers
After some discussions in the repo SIG, we moved this repository to https://github.com/jenkinsci/react-plugin-template

Gavin Mogan

unread,
Aug 22, 2019, 1:37:43 PM8/22/19
to jenkin...@googlegroups.com

A really good start, but because this group isn't very java script centric, i'm doing a fairly aggressive eye on the frontend code so when people clone the example they don't have to manually go back and fix things.

1) Why isn't this a maven jenkins arch type? Isn't that how we essentially do the starter projects?
a)  `=>` isn't portable (doesn't work in older browsers), and isn't needed there since you are not accessing "this" at all
b) originHeight gets compared to every 100 milliseconds, but never updated, so after the first time the body changes (which I assume it will as soon as content happens), it'll trigger a resize and repaint every time. Its likely the browser will be smart enough to know its the same value and not repaint, but seems easy enough to fix
c) If possible, I'd suggest looking into the inner frame using postMessage onResize to tell the outer frame it resized, that way you don't have to constantly poll every 100ms to see if its resized. Not sure if its better or not.
3) There's quite a number of references to working hours left behind. https://github.com/jenkinsci/react-plugin-template/search?utf8=%E2%9C%93&q=working&type=
4) The pom file (https://github.com/jenkinsci/react-plugin-template/blob/master/pom.xml) has a lot of unneeded dependancies left behind from working hours. all the pipeline modules. I though junit and mockito is part of parent pom. (My feeling is you can probably get away with getting rid of all of them for the boiler plate)
5) (Opinion) Also in the pom, i'd comment out the developers section saying its an example. The last thing you want is your name on everyones project asking you to fix things.
6) (Opinion) I'd probably use an officially sourced git ignore from the community project - https://gitignore.io/api/node,java (combined node and java)

For the readme, I have a couple of very minor nits.

> How does this template function?

I'm concerned about "template function" being read as template(), so maybe "how does this template work?" or "what does this template do?"

> In short, this template is like putting a webpack project inside a Maven project, and this template is just chaining the build result by copy the webpack output to the plugin's webapp folder to make it accessible from the iframe, then Jelly render the iframe and the client gets the Plugin UI.

I would drop "In Short" and "like". Its exactly that, its not like that.

> Because jenkins has last a long time, from when JSP or Jelly is widely used to render web pages, added lots of polyfills, like Prototype.js is provided to give extensions to javascript.

I'd change it to "Over time, Jenkins has added a lot of various javascript libraries to every regular page, this causes problems for using modern Javascript tooling and as such, we decided to inline the new react based pages in their own sandbox which prevents collisions with other libraries"

(i'm the least confident on the last one, but at least "jenkins has last" should be "jenkins has lasted a long time" or "jenkins has existed for a long time")

Its a really great start. I'll go over the blog item when I get some time.

Gavin


--
You received this message because you are subscribed to the Google Groups "Jenkins Developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to jenkinsci-de...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/jenkinsci-dev/26b15a4f-75e1-4f5e-a097-d187a55345b7%40googlegroups.com.

Oleg Nenashev

unread,
Aug 22, 2019, 5:33:02 PM8/22/19
to Jenkins Developers
1) Why isn't this a maven jenkins arch type? Isn't that how we essentially do the starter projects?

I marked it as a ttemplate project.
If there is a consensus that this is an approach we would like to recommend, we could move it to https://github.com/jenkinsci/archetypes

Just consider it as a WiP repo for now 

On Thursday, August 22, 2019 at 7:37:43 PM UTC+2, Gavin Mogan wrote:

A really good start, but because this group isn't very java script centric, i'm doing a fairly aggressive eye on the frontend code so when people clone the example they don't have to manually go back and fix things.

1) Why isn't this a maven jenkins arch type? Isn't that how we essentially do the starter projects?
a)  `=>` isn't portable (doesn't work in older browsers), and isn't needed there since you are not accessing "this" at all
b) originHeight gets compared to every 100 milliseconds, but never updated, so after the first time the body changes (which I assume it will as soon as content happens), it'll trigger a resize and repaint every time. Its likely the browser will be smart enough to know its the same value and not repaint, but seems easy enough to fix
c) If possible, I'd suggest looking into the inner frame using postMessage onResize to tell the outer frame it resized, that way you don't have to constantly poll every 100ms to see if its resized. Not sure if its better or not.
3) There's quite a number of references to working hours left behind. https://github.com/jenkinsci/react-plugin-template/search?utf8=%E2%9C%93&q=working&type=
4) The pom file (https://github.com/jenkinsci/react-plugin-template/blob/master/pom.xml) has a lot of unneeded dependancies left behind from working hours. all the pipeline modules. I though junit and mockito is part of parent pom. (My feeling is you can probably get away with getting rid of all of them for the boiler plate)
5) (Opinion) Also in the pom, i'd comment out the developers section saying its an example. The last thing you want is your name on everyones project asking you to fix things.
6) (Opinion) I'd probably use an officially sourced git ignore from the community project - https://gitignore.io/api/node,java (combined node and java)

For the readme, I have a couple of very minor nits.

> How does this template function?

I'm concerned about "template function" being read as template(), so maybe "how does this template work?" or "what does this template do?"

> In short, this template is like putting a webpack project inside a Maven project, and this template is just chaining the build result by copy the webpack output to the plugin's webapp folder to make it accessible from the iframe, then Jelly render the iframe and the client gets the Plugin UI.

I would drop "In Short" and "like". Its exactly that, its not like that.

> Because jenkins has last a long time, from when JSP or Jelly is widely used to render web pages, added lots of polyfills, like Prototype.js is provided to give extensions to javascript.

I'd change it to "Over time, Jenkins has added a lot of various javascript libraries to every regular page, this causes problems for using modern Javascript tooling and as such, we decided to inline the new react based pages in their own sandbox which prevents collisions with other libraries"

(i'm the least confident on the last one, but at least "jenkins has last" should be "jenkins has lasted a long time" or "jenkins has existed for a long time")

Its a really great start. I'll go over the blog item when I get some time.

Gavin


On Thu, Aug 22, 2019 at 8:48 AM Oleg Nenashev <o.v.n...@gmail.com> wrote:
After some discussions in the repo SIG, we moved this repository to https://github.com/jenkinsci/react-plugin-template
Here is also a blogpost draft: https://github.com/jenkins-infra/jenkins.io/pull/2435

On Thursday, August 22, 2019 at 11:43:42 AM UTC+2, Jack Shen wrote:
Hi,
    I had set up a boilerplate repository about using React Boilerplate to develop Jenkins Plugin, which is part of the targets of GSoC 2019 Project Working Hours Plugin UI Improvement.
    I hope this boilerplate could help Jenkins developers' way to build plugins much easier.
    The repository could be found at https://github.com/ShenJack/react-boilerplate-for-jenkins-plugin, for more detail about this boilerplate, you can read the README.md
    Thanks for reading, and any feedback are truly appreciated.

--
You received this message because you are subscribed to the Google Groups "Jenkins Developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to jenkin...@googlegroups.com.

Jesse Glick

unread,
Aug 23, 2019, 1:20:59 PM8/23/19
to Jenkins Dev
On Thu, Aug 22, 2019 at 1:37 PM 'Gavin Mogan' via Jenkins Developers
<jenkin...@googlegroups.com> wrote:
> The pom file […] has a lot of unneeded dependancies

Also (noted in Gitter) the `frontend-maven-plugin` configuration could
probably just be picked up from the `plugin-pom` parent. You just need
to create a `.mvn_exec_node` marker file IIRC.

Jack Shen

unread,
Aug 23, 2019, 1:36:16 PM8/23/19
to Jenkins Developers
I see that, would give it a try.

Jesse Glick <jgl...@cloudbees.com> 于2019年8月24日周六 上午1:21写道:
--
You received this message because you are subscribed to a topic in the Google Groups "Jenkins Developers" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/jenkinsci-dev/Xt0cZhwTkS8/unsubscribe.
To unsubscribe from this group and all its topics, send an email to jenkinsci-de...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/jenkinsci-dev/CANfRfr2Bqed4z892qOspw8ud_9xTdFWkPe%2B94to4wnY4cX8n5g%40mail.gmail.com.
Reply all
Reply to author
Forward
0 new messages