resize-observer-polyfill icon indicating copy to clipboard operation
resize-observer-polyfill copied to clipboard

Typings collide with Typescripts DOM typings for ResizeObserver

Open csvn opened this issue 3 years ago • 15 comments

Summary

The types added to global by importing 'resize-observer-polyfill' clash with Typescript's own typings for ResizeObserver in lib.dom.d.ts. This issue makes it very inconvenient to use this package.

import ResizeObserver from 'resize-observer-polyfill';

const resizeObserver = new ResizeObserver(entries => {
                                       // ^^^^^^^ Error: Parameter 'entries' implicitly has an 'any' type.
  console.log(entries);
});
// tsconfig.json
{
  "compilerOptions": {
    "strict": true,
    "lib": [
      "ES2015",
      "DOM"
    ]
  }
}

image

Suggestions

  1. Since this package is trying to be used as a ponyfill, I don't think it should affect global with it's types as soon as it's imported. It's probably better that types are exported directly from the package if they are needed. Since there already are types provided by TS, this should likely not be needed unless for some reason lib.dom.d.ts is not used.

    import ResizeObserver, { ResizeObserverEntry } from 'resize-observer-polyfill';
    // or
    import ResizeObserver from 'resize-observer-polyfill';
    import type { ResizeObserverEntry } from 'resize-observer-polyfill';
    
  2. Avoid using custom types, and rely on typescripts own types.

    // src/index.d.ts
    export default ResizeObserver;
    

csvn avatar Mar 10 '21 16:03 csvn

Can I ask what version of typescript you are using.

I asked because I update to 4.2.3 from 4.0.2 and I got the same error you got. Plus the validation is not passing for resize-observer-polyfill.

node_modules/resize-observer-polyfill/src/index.d.ts:19:18 - error TS2717: Subsequent property declarations must have the same type.  Property 'contentRect' must be of type 'DOMRectReadOnly', but here has 
type 'DOMRectReadOnly'.

19         readonly contentRect: DOMRectReadOnly;
                    ~~~~~~~~~~~

  node_modules/typescript/lib/lib.dom.d.ts:12696:14
    12696     readonly contentRect: DOMRectReadOnly;
                       ~~~~~~~~~~~
    'contentRect' was also declared here.

It seems that the resize-observer-polyfill is obsolete and typescript made the implementation.

edit: Note that the latest version of VSCode ship with 4.2.3. So your validation might pass but you would get the error when opening the file.

bmeunier1974 avatar Mar 10 '21 19:03 bmeunier1974

@bmeunier1974 yeah, I missed specifying what TS version I was using. The above is with the latest version; 4.2.3. I'm using the workspace version of Typescript, not the bundled VSCode version.

csvn avatar Mar 11 '21 19:03 csvn

You don't need to use the polyfill with Typescript 4.2.3. Removing resize-observer-polyfill should fix those errors.

bmeunier1974 avatar Mar 11 '21 19:03 bmeunier1974

Typescript does not polyfill. We need the polyfill so that older browsers get the code.

csvn avatar Mar 11 '21 19:03 csvn

Sorry, I assumed you were using it to get type definition like I did.

bmeunier1974 avatar Mar 11 '21 19:03 bmeunier1974

No problem! We're currently dynamically loading polyfills if we detect they are required, so we do something similar to this (but with several different checks and polyfills bundled together):

if (!('ResizeObserver' in window)) {
  await import('resize-observer-polyfill').then(m => window.ResizeObserver = m.default);
}

The issue is that as soon as this package is imported, it adds global interfaces, creating errors like in the issue description in other places. A workaround for now is to add explicit typing for the callback parameter, which seems to avoid the issue:

new ResizeObserver((entry: ResizeObserverEntry[]) => { ... });

csvn avatar Mar 11 '21 20:03 csvn

We target only the latest version of chrome so we don't have problem. I assume it was use for type definition. But I don't see anything why it would be needed.

But apparently there was a breaking change in TS 4.2 on how lib.d.ts are handled.

As with every TypeScript version, declarations for lib.d.ts (especially the declarations generated for web contexts), have changed. There are various changes, though Intl and ResizeObserver’s may end up being the most disruptive.

https://www.typescriptlang.org/docs/handbook/release-notes/typescript-4-2.html

bmeunier1974 avatar Mar 11 '21 20:03 bmeunier1974

Is there any update / guidance on this?

resize-observer-polyfill is not a direct dependency of our project, but is pulled in by other dependencies:

  • @vx#responsive#resize-observer-polyfill
  • antd#resize-observer-polyfill
  • antd#rc-menu#resize-observer-polyfill
  • antd#react-slick#resize-observer-polyfill
  • antd#rc-resize-observer#resize-observer-polyfill
  • antd#rc-tabs#resize-observer-polyfill
  • @sambego#storybook-state#@storybook#components#simplebar-react#simplebar#resize-observer-polyfill

We're unable to upgrade [email protected] -> 4.2.3 because of this issue:

node_modules/resize-observer-polyfill/src/index.d.ts:19:18 - error TS2717: Subsequent property declarations must have the same type.  Property 'contentRect' must be of type 'DOMRectReadOnly', but here has type 'DOMRectReadOnly'.

19         readonly contentRect: DOMRectReadOnly;
                    ~~~~~~~~~~~

  node_modules/typescript/lib/lib.dom.d.ts:12696:14
    12696     readonly contentRect: DOMRectReadOnly;
                       ~~~~~~~~~~~
    'contentRect' was also declared here.

inglec-arista avatar Mar 19 '21 18:03 inglec-arista

We encounter this issue with [email protected]. So we locked typescript to 4.2.2 version, waiting this fix.

ryuran avatar Mar 30 '21 11:03 ryuran

For those using typescript 4.2.3 that just want to get on with it while we wait for our good maintainers to prepare a more comprehensive solution, add a file at your repository root /patches/resize-observer-polyfill+1.5.1.patch with this content:

diff --git a/node_modules/resize-observer-polyfill/src/index.d.ts b/node_modules/resize-observer-polyfill/src/index.d.ts
index 74aacc0..1b236d2 100644
--- a/node_modules/resize-observer-polyfill/src/index.d.ts
+++ b/node_modules/resize-observer-polyfill/src/index.d.ts
@@ -1,14 +1,3 @@
-interface DOMRectReadOnly {
-    readonly x: number;
-    readonly y: number;
-    readonly width: number;
-    readonly height: number;
-    readonly top: number;
-    readonly right: number;
-    readonly bottom: number;
-    readonly left: number;
-}
-
 declare global {
     interface ResizeObserverCallback {
         (entries: ResizeObserverEntry[], observer: ResizeObserver): void

then add to your npm scripts

"postinstall": "npx patch-package"

YMMV, I got some errors like

 src/components/donut-chart/donut-chart.ts:192:34 - error TS2345: Argument of type '([{ contentRect: { width } }]: [{ contentRect: { width: any; }; }]) => void' is not assignable to parameter of type 'ResizeObserverCallback'.
  Types of parameters '__0' and 'entries' are incompatible.
    Type 'ResizeObserverEntry[]' is not assignable to type '[{ contentRect: { width: any; }; }]'.
      Target requires 1 element(s) but source may have fewer.

192     this.ro = new ResizeObserver(([{ contentRect: { width } }]) => this.updateScalingFactor(width));
                                     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

but that might be unrelated and part of ts 4.2.3 asserting it's "TypeScript is a superset of JavaScript" on my code style ;) I solved that one with a type assertion in the callback's params.

bennypowers avatar Apr 12 '21 13:04 bennypowers

I believe PR #85 would fix the issue.

lizraeli avatar Jul 28 '21 01:07 lizraeli

I've published a fork at https://www.npmjs.com/package/truecar-resize-observer-polyfill. The only difference is the global type fix.

This package hasn't had a release in 3 years, so you may want to look at other polyfills after this fix.

threehams avatar Aug 05 '21 17:08 threehams

Another possible workaround is to manually remap built-in definitions with own version in tsconfig.json

{
  "compilerOptions": {
    "paths": {
      "resize-observer-polyfill": ["./my-types/resize-observer-polyfill"]
    }
  }
}

and create file ./my-types/resize-observer-polyfill/index.d.ts with copy of existing types but removed DOMRectReadOnly interface definition

gbiryukov avatar Aug 26 '21 07:08 gbiryukov

I think this should be merged ASAP

danteYoon avatar Oct 28 '21 06:10 danteYoon

The last commit to this project was almost 3 years ago. I think it's unlikely that any PR will be merged. image

https://github.com/juggle/resize-observer could be an alternative. It seems to be more actively maintained (many more recent releases and commits).

csvn avatar Oct 28 '21 13:10 csvn