[Bug]: Table widget breaks when table name is renamed to Number

appsmithorg / appsmith

Platform to build admin panels, internal tools, and dashboards. Integrates with 25+ databases and any API.

https://www.appsmith.com

Apache License 2.0

34.35k stars 3.72k forks source link

[Bug]: Table widget breaks when table name is renamed to Number #9189

Open shwetha-ramesh opened 2 years ago

shwetha-ramesh commented 2 years ago

Is there an existing issue for this?

[X] I have searched the existing issues

Current Behavior

2 issues are observed when renaming the table.

Table breaks when it is renamed to a number.
When user renames the table to 2 consecutive special characters it successfully renames as '__'

https://loom.com/share/9930dff46f2141f7bf1167256a8e2566

Steps To Reproduce

Drag and drop table widget
In property pane rename the table to number and observe. 2a. In property pane rename the table to 2 consecutive special characters and observe.

Expected behavior: Table must not break when renamed to just number or must show an error message that it is not allowed.

Environment

Production

Version

Cloud

areyabhishek commented 2 years ago

@shwetha-ramesh Does this happen for every widget, API, query? Or is this only for the table widget?

shwetha-ramesh commented 2 years ago

@shwetha-ramesh Does this happen for every widget, API, query? Or is this only for the table widget?

I have observed only in table widget.

sbalaji1192 commented 2 years ago

WRT 1, Numbers are not valid identifiers in javascript. The issue exists on other widgets as well. Widget won't break but we cannot use the widget properties in js expressions. we should restrict users from renaming in this case. somewhat similar to #8881

somangshu commented 2 years ago

@sbalaji1192 is right, We should not allow numbers as widget name, I believe that is a platform level change.

cc @riodeuno

somangshu commented 2 years ago

Probably should be a high priority issue and not critical. @ajinkyakulkarni ill leave the call to you

riodeuno commented 2 years ago

@somangshu @sbalaji1192 Yes, numbers should not be the first character in identifiers. It is illegal in JS. Resolution approach:

We have functions in place which check for entity name uniqueness. (Explorer/Name.tsx uses this, so does PropertyPaneTitle.tsx)
We also have sanitiser functions which sanitise the spaces and special characters while entering the entity names (removeSpecialChars is the function name)

Using the above two features, we should disallow naming entities using numbers as the first character.

AIshwaryaZemos commented 4 months ago

I did some analysis on this issue and found that its happening only if the name starts with a digit and to resove this

we can add extra check on names(Explorer/Name.tsx), if it starts with a number or consists of only special characters then show error of invalid name.

Please let me know wheather appsmith is accepting PRs related to widgets before I proceed with the changes.