sveltejs / svelte

Cybernetically enhanced web apps
https://svelte.dev
MIT License
78.04k stars 4.08k forks source link

Incorrect derived type of store with optional content #10784

Open shadow-identity opened 5 months ago

shadow-identity commented 5 months ago

Describe the bug

If the content of a store is optional (e. g. is Something | undefined), the derived type of the store does not keep it. As a result, there is an exception in runtime, instead of error on compile/linting time.

Reproduction

  1. Add this content to a typescript file:
    let myVar: Date | undefined
    const myStore = writable(myVar)
    $myStore.toString() // for a svelte component
    get(myStore).toString() // for a plain typescript file
  2. Check the type of myStore in your editor. It is Date. No error highlighted on $myStore.toString() despite it is undefined.
  3. Run eslint and svelte-check. No errors reported.
  4. Run the code. It fails with TypeError: can't access property "toString", (intermediate value)(...) is undefined exception.

Workaround:

let myVar: Date | undefined
const myStore = writable<typeof myVar>(myVar)

Logs

No response

System Info

% pnpm dlx envinfo --system --npmPackages svelte,rollup,webpack --binaries --browsers
Packages: +1
+
Progress: resolved 1, reused 0, downloaded 1, added 1, done

  System:
    OS: macOS 14.4
    CPU: (12) x64 Intel(R) Core(TM) i7-9750H CPU @ 2.60GHz
    Memory: 42.17 MB / 16.00 GB
    Shell: 5.9 - /bin/zsh
  Binaries:
    Node: 18.14.0 - ~/.nvm/versions/node/v18.14.0/bin/node
    Yarn: 1.22.21 - ~/.nvm/versions/node/v18.14.0/bin/yarn
    npm: 9.3.1 - ~/.nvm/versions/node/v18.14.0/bin/npm
    pnpm: 8.15.4 - ~/.nvm/versions/node/v18.14.0/bin/pnpm
  Browsers:
    Chrome: 122.0.6261.112
    Chrome Canary: 112.0.5582.0
    Safari: 17.4
  npmPackages:
    svelte: ^4.2.12 => 4.2.12

Severity

annoyance

dummdidumm commented 5 months ago

We can't change this in 4.x because it would be a breaking change strictly speaking. The culprit is the type definition looking like this:

export function writable<T>(value?: T | undefined, start?: StartStopNotifier<T> | undefined): Writable<T>;

which means that because of the optional, the undefined is swallowed from the generic. For it to be preserved it seems we need to do something like this.

shadow-identity commented 2 months ago

Looks like a fix for this isn't planned for the svelte5 either?

shadow-identity commented 2 months ago

There are even worse examples of the pitfall, one with typescript:

    const myStore = writable<string>()
    const myString: string = $myStore

And another using jsdoc:

    /** @type {SvelteStore<string>} */
    const myStore = writable()

    /** @type {string} */
    const myString = $myStore

It does not produce any typing error, although myString is undefined.

It happens on svelte5 as well.

I expect type error when I try to initialize a typed store with an empty arguments list.