Bug: Private method in Stimulus controllers causes SyntaxError: Private field must be declared in an enclosing class when minified

[zachgoll]

Note: a temporary fix has been added for this issue by removing all private method syntax and using underscores instead, but it would be nice to identify the root cause of this

Describe the bug
A clear and concise description of what the bug is.

This bug only occurs in production when Stimulus controllers are minified.

For example, the linked controller produces the following error:

SyntaxError: Private field '#updateView' must be declared in an enclosing class

Here is the original file:

https://github.com/maybe-finance/maybe/blob/c5bf1db230122066571b018ed417d132e6e7e446/app/javascript/controllers/bulk_select_controller.js

And here is the minified source served in production environments:

import{Controller}from "@hotwired/stimulus"
export default class extends Controller{static targets=["row","group","selectionBar","selectionBarText","bulkEditDrawerTitle"]
static values={resource:String,selectedIds:{type:Array,default:[]}}
connect(){document.addEventListener("turbo:load",this._updateView)
this._updateView()}
disconnect(){document.removeEventListener("turbo:load",this._updateView)}
bulkEditDrawerTitleTargetConnected(element){element.innerText=`Edit ${this.selectedIdsValue.length} ${this.#pluralizedResourceName()}`}
submitBulkRequest(e){const form=e.target.closest("form");const scope=e.params.scope
this.#addHiddenFormInputsForSelectedIds(form,`${scope}[entry_ids][]`,this.selectedIdsValue)
form.requestSubmit()}
togglePageSelection(e){if(e.target.checked){this.#selectAll()}else{this.deselectAll()}}
toggleGroupSelection(e){const group=this.groupTargets.find(group=>group.contains(e.target))
this.#rowsForGroup(group).forEach(row=>{if(e.target.checked){this.#addToSelection(row.dataset.id)}else{this.#removeFromSelection(row.dataset.id)}})}
toggleRowSelection(e){if(e.target.checked){this.#addToSelection(e.target.dataset.id)}else{this.#removeFromSelection(e.target.dataset.id)}}
deselectAll(){this.selectedIdsValue=[]
this.element.querySelectorAll('input[type="checkbox"]').forEach(el=>el.checked=false)}
selectedIdsValueChanged(){this._updateView()}#addHiddenFormInputsForSelectedIds(form,paramName,transactionIds){this.#resetFormInputs(form,paramName);transactionIds.forEach(id=>{const input=document.createElement("input");input.type='hidden'
input.name=paramName
input.value=id
form.appendChild(input)})}#resetFormInputs(form,paramName){const existingInputs=form.querySelectorAll(`input[name='${paramName}']`);existingInputs.forEach((input)=>input.remove());}#rowsForGroup(group){return this.rowTargets.filter(row=>group.contains(row))}#addToSelection(idToAdd){this.selectedIdsValue=Array.from(new Set([...this.selectedIdsValue,idToAdd]))}#removeFromSelection(idToRemove){this.selectedIdsValue=this.selectedIdsValue.filter(id=>id!==idToRemove)}#selectAll(){this.selectedIdsValue=this.rowTargets.map(t=>t.dataset.id)}
_updateView=()=>{this.#updateSelectionBar()
this.#updateGroups()
this.#updateRows()}#updateSelectionBar(){const count=this.selectedIdsValue.length
this.selectionBarTextTarget.innerText=`${count} ${this.#pluralizedResourceName()} selected`
this.selectionBarTarget.hidden=count===0
this.selectionBarTarget.querySelector("input[type='checkbox']").checked=count>0}#pluralizedResourceName(){return `${this.resourceValue}${this.selectedIdsValue.length===1?"":"s"}`}#updateGroups(){this.groupTargets.forEach(group=>{const rows=this.rowTargets.filter(row=>group.contains(row))
const groupSelected=rows.length>0&&rows.every(row=>this.selectedIdsValue.includes(row.dataset.id))
group.querySelector("input[type='checkbox']").checked=groupSelected})}#updateRows(){this.rowTargets.forEach(row=>{row.checked=this.selectedIdsValue.includes(row.dataset.id)})}}

Expected behavior
A clear and concise description of what you expected to happen.

Private methods should be supported in all modern browsers.

Additional context
Add any other context about the problem here.

[gariasf]

It looks like the private field declarations get deleted upon minification. This could explain the issue, but then it wouldn't explain why the pie chart, which has a similar controller with private fields, does work.

[zachgoll]

@gariasf exactly. The pie chart controller appears (for now) to work, which invalidates a lot of my initial thoughts on why this causes errors:

https://github.com/maybe-finance/maybe/blob/e357c0485f2a227ec1a35266ef9cfffa569c653d/app/javascript/controllers/pie_chart_controller.js

[gariasf]

@zachgoll There is one difference between the scripts, the one that breaks, the line chart controller, is missing semicolons on the imports.

[gariasf]

Oh, and the static object with the values is missing the semicolon as well. That should not be an issue, though…

[zachgoll]

@gariasf good eye! I wasn't even paying attention to the imports; too distracted by the code itself! The pie chart controller does have semicolons on the imports, which explains why that file works fine when minified.

I think ultimately what this tells me is that we need to start linting these controller files in CI / auto-formatting locally :)

[oxdev03]

@gariasf good eye! I wasn't even paying attention to the imports; too distracted by the code itself! The pie chart controller does have semicolons on the imports, which explains why that file works fine when minified.

I think ultimately what this tells me is that we need to start linting these controller files in CI / auto-formatting locally :)

created #1299 for this

[zachgoll]

Fixed via #1299