Add note about typescript module augmentation

[axelson]

If the declaration is in a .d.ts file instead then you get the following error:

Module '"valtio"' has no exported member 'proxy'.ts(2305)

Here's a code sandbox that illustrates the issue: https://codesandbox.io/s/react-typescript-forked-8ijjyy?file=/src/App.tsx

[vercel[bot]]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Updated
valtio	✅ Ready (Inspect)	Visit Preview	May 15, 2022 at 1:47AM (UTC)

[codesandbox-ci[bot]]

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit d3c8fb60edfc48cef8db681f0f2f9f75cf4baece:

Sandbox	Source
React	Configuration
React Typescript	Configuration
React Browserify	Configuration
React Snowpack	Configuration
React Parcel	Configuration
React Typescript (forked)	PR

[dai-shi]

Hi, thanks for the suggestion.

https://codesandbox.io/s/react-typescript-forked-63t5ic?file=/src/decs.ts I tried renaming to *.ts, but it still shows the type error. How did you solve the issue?

[axelson]

I didn't do anything special to solve the issue. In my actual project valtio simply moving the declare module "valtio" to a js/types.ts worked fine. I'm not sure why it's not working in codesandbox.

[dai-shi]

Hmm, okay, so I did some more trials. Looks like what's important is if you import the file or not.

https://codesandbox.io/s/react-typescript-forked-tmmij0?file=/src/decs.d.ts See that ☝️ , and it works with *.d.ts.

[magicdawn]

what i see is that, this augmentation should be "augmentation" when add augmentation code in a .d.ts file, vscode complains that "can not find export proxy from valtio".

I solved this by add such code to app entry index.tsx

// augment
import 'valtio'
declare module 'valtio' {
  function useSnapshot<T extends object>(p: T): T
}

[dai-shi]

https://www.typescriptlang.org/docs/handbook/module-resolution.html I think some configs in tsconfig.json would solve the issue too even with d.ts. Can anyone investigate? Maybe this? https://www.typescriptlang.org/tsconfig#paths

[magicdawn]

my local environment

I guess vscode is confusing the declare statement is

• a "module augmentation" https://www.typescriptlang.org/docs/handbook/declaration-merging.html#module-augmentation
• or "ambient modules" https://www.typescriptlang.org/docs/handbook/modules.html#ambient-modules

csb

https://codesandbox.io/s/react-typescript-forked-8y04wn?file=/src/decs.d.ts

• i deleted the import type {xx} from './desc.d.ts'
• with / without the import 'valtio', no errors occurs

---

weird.

[macarie]

It looks like TS has issues augmenting re-exported stuff - see https://github.com/microsoft/TypeScript/issues/12607. Would be great if we could get a response from the team 👀

---

Anyway, @dai-shi do you think it might be better to allow direct access to valtio/react through the exports?

If valtio/react is added to the exports (like valtio/vanilla), then we probably could augment the module definition using

declare module 'valtio/react' {
  export function useSnapshot<T extends object>(p: T): T
}

The other option (that might be easier for everyone at this point) would be to directly export a useLooseSnapshot/useUnsafeSnapshot or something.

[dai-shi]

It looks like TS has issues augmenting re-exported stuff

So the example in https://github.com/pmndrs/valtio/pull/458#issuecomment-1136503615 isn't working correctly, but overwriting the type?

do you think it might be better to allow direct access to valtio/react through the exports?

Hmm, no, I would like to avoid such a workaround. My suggestion is not to depend on module augmentation, but do type assertion.

    const snap = useSnapshot(state) as typeof state;

You can define useLooseSnapshot on your end likewise.

const useLooseSnapshot = <T extends object>(state: T) => useSnapshot(state) as T;

[macarie]

So the example in #458 (comment) isn't working correctly, but overwriting the type?

Unfortunately, on my end at least, it's not working correctly 😵‍💫

Hmm, no, I would like to avoid such a workaround. My suggestion is not to depend on module augmentation, but do type assertion.

    const snap = useSnapshot(state) as typeof state;

You can define useLooseSnapshot on your end likewise.

const useLooseSnapshot = <T extends object>(state: T) => useSnapshot(state) as T;

Thanks for the advice! I think I'll go with the local useLooseSnapshot as it might be easier to handle in a team environment and less error-prone.

[cqh963852]

Overwriting is work for me.

---

Personal Opinion.

I don't want use a useLooseSnapshot alias.

Because the data is the same, but the constraints are different. The team will produces two practice scenarios.

Team members may get confused or complain about inconsistent spelling.

---

There are some libraries that recommend users to override the type.

https://next-auth.js.org/getting-started/typescript#extend-default-interface-properties

[dai-shi]

It's great to see various opinions. I think what works works.

Thanks guys. Closing this as what we have now seems valid. Please feel free to open a new discussion.

[magicdawn]

Update 2023, after reading TypeScript-Handbook new modules part https://www.typescriptlang.org/docs/handbook/modules/reference.html#ambient-modules

Ambient module declarations are easy to confuse with module augmentations since they use identical syntax. This module declaration syntax becomes a module augmentation when the file is a module, meaning it has a top-level import or export statement (or is affected by --moduleDetection force or auto):

as gif shows, any top level export / import make .d.ts become module augmentation. so if u are using a separate d.ts file + code as readme shows, make sure add a top level export {}

[dai-shi]

Thanks. If you have a suggestion to update README, please feel free to open a PR.

[frozencap]

@magicdawn 's suggestion should be added to the README for sure. Just lost a few hours at this