Skip to content

Commit

Permalink
[JENKINS-71909]: Revert #79
Browse files Browse the repository at this point in the history
The merge request mentions JENKINS-71365 (prototype.JS is removed from Jenkins core)
but reverting this should not impact that.

What we are doing is removing the async calls and promises, and instead using the
synchronous code again. This is so that Active Choices parameters are rendering in
order as they were before.

Whilst rendering them asynchronously seemed like a good idea to provide a more
responsive UI, in reality we oversaw an important aspect: users write their scripts
relying on the order parameters are loaded.

Users wrote their code using document.getElementId, for instance, which worked previously
as users would put the parameter doing that after the parameter from which the ID was
being used.

There were several issues rendered, so this is not likely to be added back again
any time soon. In order to have a more smart reactivity, we would need places to hook
user callback code (like Vue or React do), but provided by Jenkins UI, or in a way
that could take some time to create in the plug-in without breaking things, and that
works well.
  • Loading branch information
kinow committed Oct 6, 2024
1 parent 8cf5596 commit 8f2899f
Show file tree
Hide file tree
Showing 16 changed files with 688 additions and 231 deletions.
1 change: 1 addition & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
- Bump typescript from 5.4.3 to 5.6.2
- Bump webpack from 5.91.0 to 5.94.0
- Bump ws from 8.17.0 to 8.17.1
- Reverted JENKINS-71365, pull request #79, making the Stapler proxy synchronous again, to ensure determinism in parameters resolution, fixing a regression
- Update pom.xml to switch from node 18.16 to 18.18 (for eslint 9)
- Update pom.xml to bump Jenkins version to Jenkins 2.462.2 (job-dsl requirement)

Expand Down
2 changes: 1 addition & 1 deletion pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@
<gitHubRepo>jenkinsci/active-choices-plugin</gitHubRepo>
<node.version>18.18.0</node.version>
<yarn.version>1.22.19</yarn.version>
<ui.loading.timeout>PT60S</ui.loading.timeout>
<ui.loading.timeout>PT300S</ui.loading.timeout>
</properties>

<scm>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,13 +48,13 @@
</j:choose>
</select>
</f:entry>
<f:entry title="${%Referenced parameters}" field="referencedParameters" help="${rootURL}/../plugin/uno-choice/help-referencedParameters.html">
<f:entry title="${%Referenced parameters}" field="referencedParameters" help="/plugin/uno-choice/help-referencedParameters.html">
<f:textbox name="parameter.referencedParameters" value="${instance.referencedParameters}" />
</f:entry>
<f:entry title="${%Enable filters}" field="filterable" help="${rootURL}/../plugin/uno-choice/help-filterableParameters.html">
<f:entry title="${%Enable filters}" field="filterable" help="/plugin/uno-choice/help-filterableParameters.html">
<f:checkbox name="parameter.filterable" checked="${instance.filterable}">${%Filterable}</f:checkbox>
</f:entry>
<f:entry title="${%Filter starts at}" field="filterLength" help="${rootURL}/../plugin/uno-choice/help-filterLengthParameters.html">
<f:entry title="${%Filter starts at}" field="filterLength" help="/plugin/uno-choice/help-filterLengthParameters.html">
<f:textbox name="parameter.filterLength" default="1" value="${instance.filterLength}" />
</f:entry>
</j:jelly>
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,18 @@
// source, references table
var referencedParameters = Array();
<j:forEach var="value" items="${it.getReferencedParametersAsArray()}">
// add the element we want to monitor
referencedParameters.push("${value}");
// add the element we want to monitor
referencedParameters.push("${value}");
</j:forEach>

if (window.makeStaplerProxy) {
window.__old__makeStaplerProxy = window.makeStaplerProxy;
window.makeStaplerProxy = UnoChoice.makeStaplerProxy2;
}
var cascadeChoiceParameter = <st:bind value="${it}"/>; // Create Jenkins proxy
if (window.makeStaplerProxy) {
window.makeStaplerProxy = window.__old__makeStaplerProxy;
}
UnoChoice.renderCascadeChoiceParameter('#${h.escape(paramName)}', ${it.filterable}, '${h.escape(it.getName())}', '${h.escape(it.getRandomName())}', ${it.getFilterLength()}, '${h.escape(paramName)}', referencedParameters, cascadeChoiceParameter);

</script>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,10 +48,10 @@
</j:choose>
</select>
</f:entry>
<f:entry title="${%Enable filters}" field="filterable" help="${rootURL}/../plugin/uno-choice/help-filterableParameters.html">
<f:entry title="${%Enable filters}" field="filterable" help="/plugin/uno-choice/help-filterableParameters.html">
<f:checkbox name="parameter.filterable" checked="${instance.filterable}">${%Filterable}</f:checkbox>
</f:entry>
<f:entry title="${%Filter starts at}" field="filterLength" help="${rootURL}/../plugin/uno-choice/help-filterLengthParameters.html">
<f:entry title="${%Filter starts at}" field="filterLength" help="/plugin/uno-choice/help-filterLengthParameters.html">
<f:textbox name="parameter.filterLength" default="1" value="${instance.filterLength}" />
</f:entry>
</j:jelly>
Original file line number Diff line number Diff line change
Expand Up @@ -56,11 +56,11 @@
</j:choose>
</select>
</f:entry>
<f:entry title="${%Referenced parameters}" field="referencedParameters" help="${rootURL}/../plugin/uno-choice/help-referencedParameters.html">
<f:entry title="${%Referenced parameters}" field="referencedParameters" help="/plugin/uno-choice/help-referencedParameters.html">
<f:textbox name="parameter.referencedParameters" value="${instance.referencedParameters}" />
</f:entry>
<f:advanced>
<f:entry title="${%Omit value field}" field="omitValueField" help="${rootURL}/../plugin/uno-choice/help-omitValueField.html">
<f:entry title="${%Omit value field}" field="omitValueField" help="/plugin/uno-choice/help-omitValueField.html">
<f:checkbox name="parameter.omitValueField" checked="${instance.omitValueField}" default="false">${%Omit value field}</f:checkbox>
</f:entry>
</f:advanced>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,10 +60,18 @@
// source, references table
var referencedParameters = Array();
<j:forEach var="value" items="${it.getReferencedParametersAsArray()}">
// add the element we want to monitor
referencedParameters.push("${value}");
// add the element we want to monitor
referencedParameters.push("${value}");
</j:forEach>

if (window.makeStaplerProxy) {
window.__old__makeStaplerProxy = window.makeStaplerProxy;
window.makeStaplerProxy = UnoChoice.makeStaplerProxy2;
}
var dynamicReferenceParameter = <st:bind value="${it}"/>; // Create Jenkins proxy
if (window.makeStaplerProxy) {
window.makeStaplerProxy = window.__old__makeStaplerProxy;
}
UnoChoice.renderDynamicRenderParameter('#${paramName}', '${h.escape(it.getName())}', '${h.escape(paramName)}', referencedParameters, dynamicReferenceParameter);

// update spinner id
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@
maxCount = ${it.visibleItemCount};
}
var refElement = document.getElementById("${id}");
var refElement = document.getElementById("ecp_${h.escape(it.randomName)}_0");
if(maxCount > 0 && refElement && refElement.offsetHeight !=0) {
for(var i=0; i< maxCount; i++) {
height += refElement.offsetHeight + 3;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
<f:property field="scriptlerBuilder"/>
<f:entry
field="isSandboxed"
help="${rootURL}/../descriptor/org.jenkinsci.plugins.scriptsecurity.sandbox.groovy.SecureGroovyScript/help/sandbox"
help="/descriptor/org.jenkinsci.plugins.scriptsecurity.sandbox.groovy.SecureGroovyScript/help/sandbox"
>
<f:checkbox
title="Use Groovy Sandbox"
Expand Down
Loading

0 comments on commit 8f2899f

Please sign in to comment.