Add front-end linter and formatter

[ghassanachi]

Step 1 of #10.

1. Run a linter on client and a check for that in CI

Lint and Format

npm commands: npm run lint && npm run format

I used the configuration from npm create svelte@latest as a reference for Svelte linting and formatting best practices, since I am not familiar with it. To help organize Tailwind classes and catch conflicting classes, I added the prettier-plugin-tailwindcss package. However, I had to disable auto-loading of the prettier plugin due to a limitation. Also, I had to change tailwind.config.cjs to a CommonJS module, because it was not being loaded properly otherwise.

I fixed a few small linting and formatting errors and warnings to pass the linter. However, it might be difficult to track these changes with all the other formatting changes introduced in this PR. Here is a quick list of the fixes:

1. Remove the unused resp variable in Question.svelte
2. Remove the unused reject variable in store.js
3. Remove reject1, resolve, and reject in a few anonymous functions in store.js, since they were not being used
4. Remove the border property in a few places in the Svelte components, as the border-2 property was overriding

CI

I added a Github Actions that runs the linter and formatter for any PR into the main branch. Up to you if you want to add a branch protection rule for this action.

I considered adding Husky to run a lint and format pre-commit hook locally, but decided against it to keep things simple. If someone wants this functionality, they can create a local git-hook.

Notes

The npm commands were copied as is from the npm create svelte@latest app and target any files that they know how to format, including JSON files, configuration files, and README code blocks. I left them as is, since I don't see any harm in it. If you want to reduce the number of changes in this PR, I can restore those files and set the scripts to only target js and svelte files in our source directory.

[jonhoo]

Thanks for taking this on! It generally looks good, just some small minor notes (questions really).

[jonhoo]

Huh, why is the resolve && bit needed here?

[ghassanachi]

Eslint can't tell that resolve will always be assigned (since it is assigned inside the Promise(() => {}) function) so it complains that resolve might be undefined here.

The && is just an old habit I picked up from previous JS codebases I worked on. This code is roughly equivalent to:

// `!!` coerces resolve into a boolean, which is what the && is doing implicitly above
// !!undefined => 0
// !!(() => {})  => true  -- this works with any function
if (!!resolve) {  resolve(true) }

Since we are using ecma2020 features, I could also change it to resolve?.(true) by using the optional chaining operator.

I'm don't think there is an equivalent to the ! typescript operator (not null/undefined) in javascript, so if we want to make the linter happy without disabling it we'll need some of check (&&/if) or optional chaining.

If we want the code to be more explicit and not rely on quirks like coercion or newer features like ?, then I would probably go with if (resolve !== undefined) { resolve(true) }

[ghassanachi]

I went ahead and changed it to the explicit if (resolve !== undefined) { resolve(true) } check in 06a283d. Let me know if you would prefer the optional chaining option, or something else entirely.

[jonhoo]

My thinking here was more that if this is ever undefined, I'd want us to error, rather than just silently not call resolve the way we would given a construct like this. Tracking down failures to resolve are super painful :p

[ghassanachi]

I was sure I replied to this over the weekend, but I must have forgotten to click on comment and lost the previous message 😢.

You raised a good point, I was initially just trying to bypass this limitation of the type system (since the callback to promises is synchronous so it's not possible for resolve to ever be undefined with the current code. But I hadn't considered that future code changes could lead to frustrating troubleshooting.

The executor is called synchronously (as soon as the Promise is constructed) with the resolveFunc and rejectFunc functions as arguments. (https://developer.mozilla.org/enUS/docs/Web/JavaScript/Reference/Global_Objects/Promise/Promise)

I'll revert the code back to what it was before.

More Robust Fix Suggestion

For a more permanent fix though, the following idea came to mind:

/**
 * SAFETY: Since the Promise constructor (executor) is ran synchronously on Promise creation
 * both `resolve` and `reject` will not be `undefined` when returned from this function.
 * https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Promise/Promise
  * @returns {Promise & { resolve: (value: unkown) => void, reject: (reason?: any) => void}
 */
function deferrablePromise() {
  let resolve;
  let reject;
  const promise = new Promise((rs, rj) => {
    resolve = thisResolve;
    reject = thisReject;
  });
  return Object.assign(promise, { resolve, reject });
}
/* ... */
fetch_done = deferrablePromise(); // Create a promise that holds it's own resolve function
/* ... */ 
fetch_done.resolve(true)

The deferrablePromise would isolate the undefined issue to a single code point, and this deferrablePromise could also be used for batch, fetching and a few other places to simplify the code a little more.

[jonhoo]

That seems reasonable to me!