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.

Sign up for GitHub

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

Jump to bottom

[jsscripting] Implement NodeJS-like parameter handling for timer polyfills #15193

Merged
merged 2 commits into from
Jul 7, 2023
Merged

[jsscripting] Implement NodeJS-like parameter handling for timer polyfills #15193

merged 2 commits into from
Jul 7, 2023

Conversation

florian-h05
Copy link
Contributor

Fixes #15182.

This implements NodeJS/WebAPI-like parameter handling for the setTimeout and setInterval polyfills as documented in the MDN Web Docs.

@florian-h05
[jsscripting] Implement NodeJS-like param handling for timer polyfills
f2d0dd0 
Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
@florian-h05 florian-h05 requested a review from digitaldan July 6, 2023 16:01
@florian-h05 florian-h05 added the enhancement An enhancement or new feature for an existing add-on label Jul 6, 2023
@florian-h05 florian-h05 changed the title [jsscripting] Implement NodeJS-like param handling for timer polyfills [jsscripting] Implement NodeJS-like parameter handling for timer polyfills Jul 6, 2023
@digitaldan
Copy link
Contributor

digitaldan commented Jul 6, 2023 •
edited
Loading

Just out of curiosity , i'm looking at
https://github.com/openhab/openhab-addons/blob/7e07abbcabf679f97c7e865be3999f00a77ff2b2/bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/threading/ThreadsafeTimers.java#L169C38-L169C46

It looks like passing args should be supported there ? But the "args" argument does not look like its actually being used in the java class? I only looked at this casually, so not sure if there more to it?

@florian-h05
Copy link
Contributor Author

The Java methods only accept arguments (so we don‘t have a signature mismatch), but the arguments aren‘t actually passed to anything. Looking at the Runnable.run JavaDoc, it does not accept params. Anyway, I fear that passing the arguments through the Java layer (JS -> Java -> JS) will cause type problems.

Implementing the argument passing in the JS layer is a type-safe and really simple alternative, but I think I‘ll remove the …args params from the Java methods as they aren‘t needed anymore and can cause more confusion.

@florian-h05
[jsscripting] Clean-Up ThreadsafeTimer methods
b04b59f 
Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
digitaldan
digitaldan approved these changes Jul 7, 2023
View reviewed changes
Copy link
Contributor

@digitaldan digitaldan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@florian-h05
Copy link
Contributor Author

@jlaur Ping

jlaur
jlaur approved these changes Jul 7, 2023
View reviewed changes
Copy link
Contributor

@jlaur jlaur left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@jlaur jlaur merged commit 2d75536 into openhab:main Jul 7, 2023
@jlaur jlaur added this to the 4.0 milestone Jul 7, 2023
@florian-h05 florian-h05 deleted the jsscripting-timer-polyfills branch July 7, 2023 23:48
markus7017 pushed a commit to markus7017/openhab-addons that referenced this pull request Jul 8, 2023
@florian-h05 @markus7017
[jsscripting] Implement NodeJS-like parameter handling for timer poly…
bd41925 
…fills (openhab#15193)

* [jsscripting] Implement NodeJS-like param handling for timer polyfills
* [jsscripting] Clean-Up ThreadsafeTimer methods

Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
florian-h05 added a commit to florian-h05/openhab-addons that referenced this pull request Jul 26, 2023
@florian-h05
[jsscripting] Fix timerId not returned by JS timer methods
0dd2865 
Regression from openhab#15193.
Reported on the community, see https://community.openhab.org/t/openhab-4-0-release-discussion/147957/53?u=florian-h05.

Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
@florian-h05 florian-h05 mentioned this pull request Jul 26, 2023
Merged
jlaur pushed a commit that referenced this pull request Jul 26, 2023
@florian-h05
[jsscripting] Fix timerId not returned by JS timer methods (#15308)
e1c9213 
Regression from #15193.
Reported on the community, see https://community.openhab.org/t/openhab-4-0-release-discussion/147957/53?u=florian-h05.

Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
jlaur pushed a commit that referenced this pull request Jul 26, 2023
@florian-h05 @jlaur
[jsscripting] Fix timerId not returned by JS timer methods (#15308)
974847f 
Regression from #15193.
Reported on the community, see https://community.openhab.org/t/openhab-4-0-release-discussion/147957/53?u=florian-h05.

Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
matchews pushed a commit to matchews/openhab-addons that referenced this pull request Aug 9, 2023
@florian-h05 @matchews
[jsscripting] Implement NodeJS-like parameter handling for timer poly…
e3de42f 
…fills (openhab#15193)

* [jsscripting] Implement NodeJS-like param handling for timer polyfills
* [jsscripting] Clean-Up ThreadsafeTimer methods

Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
Signed-off-by: Matt Myers <mmyers75@icloud.com>
austvik pushed a commit to austvik/openhab-addons that referenced this pull request Mar 27, 2024
@florian-h05 @austvik
[jsscripting] Implement NodeJS-like parameter handling for timer poly…
4b0b80e 
…fills (openhab#15193)

* [jsscripting] Implement NodeJS-like param handling for timer polyfills
* [jsscripting] Clean-Up ThreadsafeTimer methods

Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
Signed-off-by: Jørgen Austvik <jaustvik@acm.org>
austvik pushed a commit to austvik/openhab-addons that referenced this pull request Mar 27, 2024
@florian-h05 @austvik
[jsscripting] Fix timerId not returned by JS timer methods (openhab#1…
ddee1c5 
…5308)

Regression from openhab#15193.
Reported on the community, see https://community.openhab.org/t/openhab-4-0-release-discussion/147957/53?u=florian-h05.

Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
Signed-off-by: Jørgen Austvik <jaustvik@acm.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jlaur jlaur jlaur approved these changes

@digitaldan digitaldan digitaldan approved these changes

Assignees
No one assigned
Labels
enhancement An enhancement or new feature for an existing add-on
Projects
None yet
Milestone
4.0
Development

Successfully merging this pull request may close these issues.

[jsscripting] third parameter of setTimeout() ignored
3 participants
@florian-h05 @digitaldan @jlaur