ErikOnBike / CodeParadise

Framework for developing web applications and Node.js applications using Smalltalk
MIT License
83 stars 9 forks source link

CpTransaction class>>performTransitions now properly removes finished transitions #19

Closed mumez closed 1 year ago

mumez commented 1 year ago

I found that after performing CpTransition, MessageNotUnderstood errors are continuously displayed in JavaScript console. 2023-03-14 01_24_31-DevTools - localhost_8080_static_app html_ChartJS-Examples

After some tracing, I found that Transitions class variable is accidentally nil in the reverseDo: block. At the latter part of CpTransaction class>>performTransitions:

finishedTransitions reverseDo: [ :each |
        (Transitions "<-strangely nil!" lastIndexOf: each startingAt: index ifAbsent: [ nil ])

It seems to be an optimization bug on the SqueakJS side. As a workaround, I split this part into other method. In the unregisterFinishedTransitions:startingFrom:, I use a currentTransitions temporary variable to refer Transitions in reverseDo: block and put some comment for this workaround.

After the change, the errors are gone and finished transitions are properly unregistered. Please review the PR!

ErikOnBike commented 1 year ago

Hi Masashi,

Thank you very much for reporting and providing a work-around.

The actual problem was slightly different than the message seemed to indicate. The actual class var did not become nil. What happened was that the index got bigger than the size of the transitions (when removing the last transition). I made an update to fix this. So I'm closing this PR (without accepting the work-around code, since it is not necessary anymore).

Warm regards, Erik

mumez commented 1 year ago

Thank you for the better fix! I appreciate it.

ErikOnBike commented 1 year ago

@mumez I have used your workaround in an update today. For some reason my previous 'fix' does not work anymore. So I decided to give your workaround a try and it does work. I have to investigate if this is a SqueakJS issue or not. Thx again for providing the workaround! :-)