rstudio / DT

R Interface to the jQuery Plug-in DataTables
https://rstudio.github.io/DT/
Other
597 stars 182 forks source link

Moved function definition #1136

Open MichalLauer opened 5 months ago

MichalLauer commented 5 months ago

Hello,

the only purpose of this PR is to change the order of function definition. Currently, processWidget() is used in function widgetFunc(), but only after it's definition. This makes the package https://github.com/rstudio/shinytest2 fail because it does not allow global variables.

I am aware that this issue is more related to {shinytest2} rather than to {DT}, but the issue there is stale (https://github.com/rstudio/shinytest2/issues/330). As this change does not affect any functionality, I thought it could be fixed right here.

If I should update also the package version/NEWS or something similar, please, feel free to let me know.

If you do not like this change, I will understand. Thanks for this package and your hard work :)

CLAassistant commented 5 months ago

CLA assistant check
All committers have signed the CLA.

jcheng5 commented 5 months ago

Wow, this is incredibly bad if the order matters like this. Do you have a reprex that triggers on this particular issue? I don't see anything in https://github.com/rstudio/shinytest2/issues/330.

MichalLauer commented 5 months ago

I tried to create a reprex but it looks like this issue happens only in a package structure. I created a repository where the issue is replicable: https://github.com/MichalLauer/DTReprex.

To replicate, you need to: 1) clone the repository git clone git@github.com:MichalLauer/DTReprex.git 2) open the test.R file 3) run all

It's strange that you really don't need to pass any data to it - the error happens with any DT settings.