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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Typings collide with Typescripts DOM typings for ResizeObserver #83

Open
csvn opened this issue Mar 10, 2021 · 15 comments
Open

Typings collide with Typescripts DOM typings for ResizeObserver #83

csvn opened this issue Mar 10, 2021 · 15 comments

Comments

@csvn
Copy link

csvn commented Mar 10, 2021

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"
    ]
  }
}
  • typescript@4.2.3
  • resize-observer-polyfill@1.5.1

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;
@bmeunier1974
Copy link

bmeunier1974 commented Mar 10, 2021

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.

@csvn
Copy link
Author

csvn commented Mar 11, 2021

@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.

@bmeunier1974
Copy link

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

@csvn
Copy link
Author

csvn commented Mar 11, 2021

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

@bmeunier1974
Copy link

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

@csvn
Copy link
Author

csvn commented Mar 11, 2021

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[]) => { ... });

@bmeunier1974
Copy link

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

@inglec-arista
Copy link

inglec-arista commented Mar 19, 2021

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 typescript@4.1.5 -> 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.

@ryuran
Copy link

ryuran commented Mar 30, 2021

We encounter this issue with typescript@4.2.3.
So we locked typescript to 4.2.2 version, waiting this fix.

@bennypowers
Copy link

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.

@lizraeli
Copy link

lizraeli commented Jul 28, 2021

I believe PR #85 would fix the issue.

@threehams
Copy link

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.

@gbiryukov
Copy link

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

Philipp91 added a commit to Philipp91/react-resize-detector that referenced this issue Oct 22, 2021
> Handle element resizes like it's 2021!

In 2021, it's already been 1-3 years since major browsers started supporting ResizeObserver natively. Users who haven't updated their browser for that long are in trouble anyway. Application developers who still want to support such users, can just add back the dependency. The latest version of the polyfill has broken types (que-etc/resize-observer-polyfill#83), so it's a hassle to deal with.

This requries a major version release.
Philipp91 added a commit to Philipp91/react-resize-detector that referenced this issue Oct 22, 2021
> Handle element resizes like it's 2021!

In 2021, it's already been 1-3 years since major browsers started supporting ResizeObserver natively. Users who haven't updated their browser for that long are in trouble anyway. Application developers who still want to support such users, can just add back the dependency. The latest version of the polyfill has broken types (que-etc/resize-observer-polyfill#83), so it's a hassle to deal with.

This requries a major version release.
Philipp91 added a commit to Philipp91/react-resize-detector that referenced this issue Oct 22, 2021
> Handle element resizes like it's 2021!

In 2021, it's already been 1-3 years since major browsers started supporting ResizeObserver natively. Users who haven't updated their browser for that long are in trouble anyway. Application developers who still want to support such users, can just add back the dependency. The latest version of the polyfill has broken types (que-etc/resize-observer-polyfill#83), so it's a hassle to deal with.

This requries a major version release.
@danteYoon
Copy link

I think this should be merged ASAP

@csvn
Copy link
Author

csvn commented Oct 28, 2021

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).

AMoo-Miki added a commit to AMoo-Miki/OpenSearch-Dashboards that referenced this issue Feb 11, 2022
* Fix components that use deprecated EUI methods and properties
  * Remove `withTitle`
  * Remove `compressed` from `EuiFormRow`
  * Replace `compressed` with `formRowDisplay` on `ValidatedDualRange`
* Apply newer EUI definitions to complaining components
* Replace `resize-observer-polyfill` with `@juggle/resize-observer` and `@types/resize-observer-browser` due to [typing collision](que-etc/resize-observer-polyfill#83)

Signed-off-by: Miki <miki@amazon.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

9 participants