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

Seems to not work with resize-observer-polyfill #7

Closed
eniovalo opened this issue May 14, 2020 · 10 comments
Closed

Seems to not work with resize-observer-polyfill #7

eniovalo opened this issue May 14, 2020 · 10 comments

Comments

@eniovalo
Copy link

Hi,
I was trying to include resize-observer-polyfill, but it seems to not work. I think this plugin or rollup is adding $1 at the name of the polyfill.
Example: ResizeObserver turns to ResizeObserver$1

I used rollup-plugin-inject or an Import import ResizeObserver from 'resize-observer-polyfill' but I think it would be more legible if I use rollup-plugin-polyfill. ;)

Below some code of my tests:
Thanks.

Code of ResizeObserver Polyfill

/**
 * ResizeObserver API. Encapsulates the ResizeObserver SPI implementation
 * exposing only those methods and properties that are defined in the spec.
 */
var ResizeObserver = /** @class */ (function () {
    /**
     * Creates a new instance of ResizeObserver.
     *
     * @param {ResizeObserverCallback} callback - Callback that is invoked when
     *      dimensions of the observed elements change.
     */
    function ResizeObserver(callback) {
        if (!(this instanceof ResizeObserver)) {
            throw new TypeError('Cannot call a class as a function.');
        }
        if (!arguments.length) {
            throw new TypeError('1 argument required, but only 0 present.');
        }
        var controller = ResizeObserverController.getInstance();
        var observer = new ResizeObserverSPI(callback, controller, this);
        observers.set(this, observer);
    }
    return ResizeObserver;
}());

Using rollup-plugin-polyfill

Rollup config:

{
	input,
	plugins: [
		polyfill(['resize-observer-polyfill', './platform/platform.dom.js']),
		json(),
		resolve(),
		babel(),
		cleanup({
			sourcemap: true
		})
	],
	output: {
		name: 'Chart',
		file: 'dist/Chart.js',
		banner,
		format: 'umd',
		indent: false,
	},
},

Generated file after rollup - Polyfill:

var ResizeObserver$1 =
  function () {
    function ResizeObserver(callback) {
      if (!(this instanceof ResizeObserver)) {
        throw new TypeError('Cannot call a class as a function.');
      }
      if (!arguments.length) {
        throw new TypeError('1 argument required, but only 0 present.');
      }
      var controller = ResizeObserverController.getInstance();
      var observer = new ResizeObserverSPI(callback, controller, this);
      observers.set(this, observer);
    }
    return ResizeObserver;
  }();

var index = function () {
    if (typeof global$1.ResizeObserver !== 'undefined') {
      return global$1.ResizeObserver;
    }
    return ResizeObserver$1;
}();

Generated file after rollup - Usage:

var observer = new ResizeObserver(function (entries) {
  var entry = entries[0];
  resize(entry.contentRect.width, entry.contentRect.height);
});

It results at ResizeObserver is not defined because rollup generated a polyfill named ResizeObserver$1 insted of ResizeObserver

Importing directly the polyfill

Rollup config:

{
	input,
	plugins: [
		polyfill(['./platform/platform.dom.js']),
		json(),
		resolve(),
		babel(),
		cleanup({
			sourcemap: true
		})
	],
	output: {
		name: 'Chart',
		file: 'dist/Chart.js',
		banner,
		format: 'umd',
		indent: false,
	},
},

Importing ResizeObserver Polyfill directly in the code:

import ResizeObserver from 'resize-observer-polyfill';

const observer = new ResizeObserver(entries => {
	const entry = entries[0];
	resize(entry.contentRect.width, entry.contentRect.height);
});

Using rollup-plugin-inject

Rollup config:

const inject = require('@rollup/plugin-inject');
{
	input,
	plugins: [
		// set default because is the default export.
		inject({
			ResizeObserver: ['resize-observer-polyfill', 'default']
		}),
		polyfill(['./platform/platform.dom.js']),
		json(),
		resolve(),
		babel(),
		cleanup({
			sourcemap: true
		})
	],
	output: {
		name: 'Chart',
		file: 'dist/Chart.js',
		banner,
		format: 'umd',
		indent: false,
	},
},

Importing directly and rollup-plugin-inject generate the same code that works

Generated file after rollup - Polyfill:

var ResizeObserver =
  function () {
    function ResizeObserver(callback) {
      if (!(this instanceof ResizeObserver)) {
        throw new TypeError('Cannot call a class as a function.');
      }
      if (!arguments.length) {
        throw new TypeError('1 argument required, but only 0 present.');
      }
      var controller = ResizeObserverController.getInstance();
      var observer = new ResizeObserverSPI(callback, controller, this);
      observers.set(this, observer);
    }
    return ResizeObserver;
  }();

var index = function () {
    if (typeof global$1.ResizeObserver !== 'undefined') {
      return global$1.ResizeObserver;
    }
    return ResizeObserver;
}();

Generated file after rollup - Usage:

var observer = new index(function (entries) {
  var entry = entries[0];
  resize(entry.contentRect.width, entry.contentRect.height);
});
@JRJurman
Copy link
Owner

@eniovalo just saw this now, sorry for the delay!
Thanks for detailing everything you are expecting / seeing. It should help me re-create the issue on my end, and I'll see what I can come up with for a fix!

@JRJurman
Copy link
Owner

JRJurman commented May 26, 2020

@eniovalo I tried re-creating the issue on my end, but I'm not seeing it the $1 after the ResizeObserver. I'm just editing the example in the example folder... I wonder if the babel step is causing the suffix?

Do you think you could create a minimal setup (either based on the example folder in the project or as a brand new gist / github repo), that shows this issue? I want to be sure I can recreate the issue on my machine before I start debugging more.

@eniovalo
Copy link
Author

@eniovalo just saw this now, sorry for the delay!
Thanks for detailing everything you are expecting / seeing. It should help me re-create the issue on my end, and I'll see what I can come up with for a fix!

@JRJurman, No problems. =]

Yes, I will create a minimal setup and send to you. ;)

@eniovalo
Copy link
Author

@JRJurman , I create an example with resizeObserver, it seems that rollup-plugin-polyfill conflicts with new ResizeObserver and add $1 to the name of the function ResizeObserver$1 . I don't know why.

If you could check it, it will be grateful. =]

@JRJurman
Copy link
Owner

@eniovalo I see the $1 in the builds, but the way these functions are setup, I believe they will still put a ResizeObserver global on the window object... I tested all three builds on my Linux device, and none of them failed... I can try with IE, but this minimal setup doesn't appear to be causing the issue ResizeObserver is not defined that you were seeing... At least not on my setup - was there a specific build (iife, commonjs, or umd) that was causing this, or all of them?

@JRJurman
Copy link
Owner

Just verified that none of the builds work with IE, but that is because IE needs the code to be transpiled (probably with babel). If you can get a version that shows the error in IE (or any browser), that would be useful.

@eniovalo
Copy link
Author

@JRJurman , I tested the 3 builds (iife, commonjs and umd) with Firefox ESR 68.7 and it failed ResizeObserver is not defined because ResizeObserver is compatible since Firefox 69 and IE is not compatible. =(

error in firefox esr 68

So, I put resize-observer-polyfill with rollup-plugin-polyfill to make it compatible, but it generates ResizeObserver$1. If you change ResizeObserver$1 to ResizeObserver at bundle.js (line 893), it will work at IE. =]

resize-observer-polyfill code (before rollup):
resize-observer-polyfill

resize-observer-polyfill code (after rollup):
bundlejs

@JRJurman
Copy link
Owner

Reading the documentation for resize-observer-polyfill, it "recommends" using it as a ponyfill https://github.com/que-etc/resize-observer-polyfill#usage-example but in reality, you have to use it as a ponyfill, see que-etc/resize-observer-polyfill#68

It occurs to me that this would be incompatible with rollup-plugin-polyfill, as we assume that the polyfill should populate the global (as they usually do).

It seems that your usage (at least in the examples you've provided) are not compatible with the usage of the resize-observer-polyfill library.

Conversely, you could write a wrapper around resize-observer-polyfill which populates the global with a ResizeObserver, and then polyfill that wrapper! That should work just fine 👍

@eniovalo
Copy link
Author

@JRJurman , thanks.
I will create a wrapper to make resize-observer-polyfill global. =]

@eniovalo
Copy link
Author

eniovalo commented Jun 1, 2020

@JRJurman , rollup-plugin-polyfill worked with a wrapper of resize-observer-polyfill.
Thanks for the advice! =]

import ResizeObserver from 'resize-observer-polyfill';

//Finding the global variable.
const myGlobal = (function () {
    if (typeof global !== 'undefined' && global.Math === Math) {
        return global;
    }

    if (typeof self !== 'undefined' && self.Math === Math) {
        return self;
    }

    if (typeof window !== 'undefined' && window.Math === Math) {
        return window;
    }
})();

//If ResizeObserver doesn't exist, set the Polyfill.
if(!myGlobal.ResizeObserver) {
    myGlobal.ResizeObserver = ResizeObserver;
}

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

2 participants