Fix/viewer errors

[meganrm]

Problem

The test bed viewer had a lot of errors, and it loaded files different than the web app, ie you had to click "load".

Solution

I loaded everything from the local files, and changed the "on change" function to just load the file immediately.

Type of change

Please delete options that are not relevant.

• Bug fix (non-breaking change which fixes an issue)

[github-actions]

jest coverage report 🧪

Total coverage

Status	Category	Percentage	Covered / Total
🔴	Statements	40.14%	1965/4895
🔴	Branches	43.92%	821/1869
🔴	Functions	37.22%	405/1088
🔴	Lines	40.39%	1882/4659

Status of coverage: 🟢 - ok, 🟡 - slightly more than threshold, 🔴 - under the threshold

[toloudis]

some of this is already in #341

[toloudis]

The original from "../es" was there intentionally to help simulate an environment more like simularium-website where we are loading from the packaged build. This was relevant because of differences in the way the web worker and style sheets are loaded, for example. So if changing this, we should be intentional about it

[meganrm]

ok, it seemed like this was what was causing a lot of the typing errors and it means you don't have to rebuild to make sure all your changes are reflected in the app. I don't see any differences after changes these to the way the app behaves. do you know of something I could test?

[meganrm]

I had to change the server to production because the staging one is down, but I was able to load COVIDLUNG and it seemed to behave the same in both cases (changing the imports)

[toloudis]

This is expected to behave the same if you :

• use from ../es and run npm build before npm start
• compared to use from ../src and don't build before start.

Main thing to test is loading a trajectory with pdbs.
But the ../es case is more important to make sure things are working for production.
Being intentional probably means just keeping it easy to switch between the two (keep all the imports in this one statement if possible) and maybe put something in the readme, and a comment at the import to say what to do.

[meganrm]

no I know, I meant is there any behavior you know about that does behave differently in the two states? Because either way we're running the same commands, ie, npm start calls npm build and the dev server setup is supposed to serve the built artifacts just while watching for changes. I'm in favor of using the /src because it removes the typescript errors so it's more obvious when we do have an actual error in the test bed viewer. But I can add a commented out import statement? Is that what you're suggesting?

[toloudis]

There should be no actual differences in runtime behavior between the two cases of importing from src vs es. In the past, however, there HAVE been differences which had to be fixed. Loading it here lets you find the error before you test in simularium-website. This is the reason why this demo app is loading from es/.

There definitely shouldn't be any typescript errors when we import from the es/ dir, so if there are, maybe that means we need to make sure that the test app can see the type-declarations directory.

[toloudis]

So I would like to keep the ability to run the test app with the post-build artifacts.

In order to support both situations (import from es and requiring a npm build, versus importing from src and not requiring a npm build) I would suggest a separate start script in package.json and some kind of dev-or-production switch in the code. The alternative is to have a commented-out version of both like this:

import foo, bar, baz from 'src';
// import foo, bar, baz from 'es';

As long as there is only one place to do this, then you have to just change the comment to make it use the right thing. And then every PR from now on will have this comment/uncomment battle... 🤣

[ShrimpCryptid]

Looks good!