Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Compile error for TypeScript projects that transpile to ES5 #718

Closed
3 tasks done
octogonz opened this issue Dec 8, 2020 · 6 comments
Closed
3 tasks done

Compile error for TypeScript projects that transpile to ES5 #718

octogonz opened this issue Dec 8, 2020 · 6 comments

Comments

@octogonz
Copy link

octogonz commented Dec 8, 2020

馃悰 Bug Report

Some background: We are trying to adopt @reduxjs/toolkit@1.4.0, which depends on immer. Our projects target ES5 because we need to support devices with ancient JavaScript runtimes. Our applications have lots of components split across many teams and projects. So for performance reasons, we've agreed upon a standard set of polyfills that everyone uses. The polyfills are loaded manually, and transpilation is performed by the TypeScript compiler instead of Babel.

Our build configuration does not support the Proxy polyfill. (It's not a commonly needed API, and that particular polyfill provides only partial functionality anyway.)

This causes a compile error for the immer typings:

node_modules/immer/dist/core/proxy.d.ts:31:35 - error TS2304: Cannot find name 'ProxyHandler'.

Immer currently seems to have a hard dependency on ES2015.Proxy in tsconfig.json.

To Reproduce

  1. Clone https://github.com/octogonz/immer-repro
  2. npm install
  3. npm run build

Suggested fix

The error is caused by this definition:

immer/dist/core/proxy.d.ts

export declare const objectTraps: ProxyHandler<ProxyState>;

If that one line is commented out, then the project builds successfully.

I tried fixing it by defining something like this in our application, however this seems to augment the global declarations only for our project's scope; immer's proxy.d.ts still fails to compile:

declare global {
  // This eliminates the compile error if pasted directly into proxy.d.ts,
  // but does not work when declared via my own project.
  export type ProxyHandler<ProxyState> = unknown;
}

But here's a couple possible fixes that would work:

  1. Move objectTraps into a separate "opt-in" entry point. Thus this declaration would only be compiled by projects that import an Immer API that uses it.
  2. Keep objectTraps where it is, but modify its type declaration to avoid depending on the system ProxyHandler definition. For example, if you simply paste the compiler's ProxyHandler interface declaration into immer/dist/core/proxy.d.ts, then everything works whether or not ES2015.Proxy is added to tsconfig.json.

Environment

We only accept bug reports against the latest Immer version.

  • Immer version: 8.0.0
  • I filed this report against the latest version of Immer
  • Occurs with setUseProxies(true)
  • Occurs with setUseProxies(false) (ES5 only) 馃憠 irrelevant, since this is a compile-time error

CC @victor-wm @bartvandenende-wm

@mweststrate
Copy link
Collaborator

I'd recommend to set skipLibCheck in your tsconfig if you're using a less common config, to avoid issues with Immer and potentially many other libs. There is no reason your tsc should typecheck our already compiled code again, especially not with a different configuration. (Imho this option should just be the default)

I think your global declaration should might work as well if you put it as any instead of unknown?

@Bnaya
Copy link

Bnaya commented Dec 10, 2020

Screen Shot 2020-12-10 at 13 58 15
This is the only problematic symbol and it's not part of the public api,
It is possible to restructure the files and make this specific symbol not effecting consuming programs.
Or we may add /// to make consuming programs load Proxy types
https://www.typescriptlang.org/docs/handbook/triple-slash-directives.html#-reference-lib-

@Bnaya
Copy link

Bnaya commented Dec 10, 2020

I've made a possible fix: #721
@octogonz you may try the version from the codesandbox ci: https://pkg.csb.dev/immerjs/immer/commit/abb4e3c4/immer

@octogonz
Copy link
Author

@Bnaya I have confirmed that the codesandbox release eliminates the compile error. Thank you very much for fixing it so quickly!

@octogonz
Copy link
Author

I'd recommend to set skipLibCheck in your tsconfig if you're using a less common config, to avoid issues with Immer and potentially many other libs. There is no reason your tsc should typecheck our already compiled code again, especially not with a different configuration. (Imho this option should just be the default)

The TypeScript compiler has good justification for choosing this default. If code fails to compile, it means compilation must rely on incomplete or incorrect type information when analyzing the project files. In many cases the it probably won't break anything, but "probably won't break anything" is not the bar for professional software development at work, particularly for a component as fundamental as the compiler. 馃檪

and potentially many other libs.

We use thousands of other NPM packages in our TypeScript repos -- none of them have compile errors.

@mweststrate
Copy link
Collaborator

Since there have been no additional reports of this issue in two years, and Proxy supporting browsers are nowadays quite the norm, won't fix this issue. skipLibCheck or a global ProxyHandlerDeclare, or patch-package can be used as workaround.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants