Describe the bug
When localStorage contains a reference to an unexisting parser, the whole AstExplorer app crashes.
To Reproduce
Steps to reproduce the behavior:
Check out some PR that adds a new parser (e.g. #682)
Build and run AstExplorer locally
Select the new parser in AstExplorer UI
Switch back to master branch
Rebuild and run AstExplorer
Refresh the browser window
Observe the page crashing
Expected behavior
No crash.
Screenshots
Browser:
I experienced it in MacOS 12.6 Firefox 107.0.1, but I'm confident the problem is the same in all browsers.
astexplorer settings:
Selected parser: pgsql-ast-parser
Contents of the local storage key explorerSettingsV1: {"showTransformPanel":false,"parserSettings":{"sql-parser-cst":{"dialect":"bigquery","preserveComments":true,"includeRange":true}},"parserPerCategory":{"sql":"pgsql-ast-parser"},"workbench":{"parser":"pgsql-ast-parser","code":"SELECT * FROM tbl\n","keyMap":"default","transform":{"code":"","transformer":null}}}
The main culprit seems to be the value stored in workbench.parser field. When I change it to an existing parser, the app starts working.
Additional context
The invalid parser name can end up in localStorage in a few ways:
When working on a branch that adds new parser to AstExplorer and then switching back to master branch.
When the parser with that name used to be available in AstExplorer, but has been removed (or was renamed).
The first one is likely the most common scenario, and it's a very surprising and hard to debug behavior. One usually expects that switching back to the master will get one to a good state, but here the opposite happens - the feature branch is working, but somehow the master is broken. Cleaning node_modules and reinstalling all won't help you. Doing a whole clean checkout of the repository also won't fix it.
Today I spent a few hours trying to figure out what the heck is going wrong. I knew that master had been in working order before, but now it was broken, and I had no idea why. Even though I had actually experienced the exact same issue a few weeks ago and sorted it out, I had already forgotten about it and had to figure it all out again.
The second scenario is more of a possibility, as it seems that parsers mostly get added here. But I bet that at one day there will be a situation where a parser needs to be removed (e.g. it contains a serious vulnerability) or needs to be renamed. At which point this problem will come to bite regular users.
I found some other issues which hint at the same problem or at least something very similar:
Describe the bug When localStorage contains a reference to an unexisting parser, the whole AstExplorer app crashes.
To Reproduce Steps to reproduce the behavior:
Expected behavior No crash.
Screenshots
Browser:
astexplorer settings:
pgsql-ast-parserexplorerSettingsV1:{"showTransformPanel":false,"parserSettings":{"sql-parser-cst":{"dialect":"bigquery","preserveComments":true,"includeRange":true}},"parserPerCategory":{"sql":"pgsql-ast-parser"},"workbench":{"parser":"pgsql-ast-parser","code":"SELECT * FROM tbl\n","keyMap":"default","transform":{"code":"","transformer":null}}}The main culprit seems to be the value stored in
workbench.parserfield. When I change it to an existing parser, the app starts working.Additional context
The invalid parser name can end up in localStorage in a few ways:
The first one is likely the most common scenario, and it's a very surprising and hard to debug behavior. One usually expects that switching back to the master will get one to a good state, but here the opposite happens - the feature branch is working, but somehow the master is broken. Cleaning node_modules and reinstalling all won't help you. Doing a whole clean checkout of the repository also won't fix it.
Today I spent a few hours trying to figure out what the heck is going wrong. I knew that
masterhad been in working order before, but now it was broken, and I had no idea why. Even though I had actually experienced the exact same issue a few weeks ago and sorted it out, I had already forgotten about it and had to figure it all out again.The second scenario is more of a possibility, as it seems that parsers mostly get added here. But I bet that at one day there will be a situation where a parser needs to be removed (e.g. it contains a serious vulnerability) or needs to be renamed. At which point this problem will come to bite regular users.
I found some other issues which hint at the same problem or at least something very similar:
496
448
347