iabtcf-es icon indicating copy to clipboard operation
iabtcf-es copied to clipboard

Support option to set polyfills

Open nealrosen04 opened this issue 4 years ago • 16 comments

Describe the feature you'd like I know you are inclined to not include polyfills directly and prefer to leave it to consumers of this library to define any necessary polyfills. This is fine for consumers who can control which polyfills are set on the page. But this poses a problem for CMPs using your library that have less control over the polyfills a publishers includes on their page.

Currently, the only option seems to be to have our CMP define global polyfills. This is not the preferred approach. Libraries should not set global polyfills affecting code outside of the library.

Option 2 would be for the iabtcf library to include polyfills directly. This would increase the size of the iabtcf library and may not be desired by everyone.

Option 3 (my feature request) is to expose a way for consumers to provide polyfills to the iabtcf library. For example, the consumer code to pass in polyfills might look like

import {Polyfills} from '@iabtcf/core';
import Map from 'es6-map/implement';

Polyfills.Map = Map;

and the iabtcf code would use Polyfills.Map if it exists else default to the globally available Map.

From my testing, the polyfills required for IE 10+ compatibility include

  • Promise
  • Map
  • Set
  • Symbol
  • Array.includes - only used a few times in the core. Perhaps this could be replaced with indexOf to avoid compatibility issues with IE.

nealrosen04 avatar Mar 25 '20 12:03 nealrosen04

Firstly, thanks for raising this @nealrosen04 I know that most will want a solution to this. I know there's a way to solve it. On your three options above:

  1. CMP defines Polyfills – are you sure that the only option is to have polyfills that pollute the global prototypes? I feel like this problem has been solved already in some way. https://github.com/zloirock/core-js/blob/master/README.md and I just don't know the solution.
  2. @iabtcf lib includes directly – The reason I've been reticent to do this is that if a CMP is already including PolyFills then it's code duplication. Or if they don't want it – for a Node JS environment or something – then it's just unnecessary overhead.
  3. expose way for lib users to provide PolyFills – Your proposal could work, but I've never seen it done this way so I wonder if it's the best way to solve it? I'd like to do more research, maybe you could help with that as well?

I'd like to open this up for discussion a little to see if there are any other options that people can think of?

chrispaterson avatar Mar 25 '20 17:03 chrispaterson

  1. CMP defines Polyfills – are you sure that the only option is to have polyfills that pollute the global prototypes? I feel like this problem has been solved already in some way. https://github.com/zloirock/core-js/blob/master/README.md and I just don't know the solution. -- There are 2 options, pollute the global namespace, or import the polyfilled objects into the file that uses them. Without updates to the @iabtcf lib, my only option is to pollute the global namespace.

  2. @iabtcf lib includes directly – The reason I've been reticent to do this is that if a CMP is already including PolyFills then it's code duplication. Or if they don't want it – for a Node JS environment or something – then it's just unnecessary overhead. -- that makes sense. what do you think of the option of generating 2 versions of each lib. (with and without polyfills). I realize this isn't ideal either.

  3. expose way for lib users to provide PolyFills – Your proposal could work, but I've never seen it done this way so I wonder if it's the best way to solve it? I'd like to do more research, maybe you could help with that as well? -- I haven't seen this anywhere either but it is similar to option 2. The difference is in who imports the polyfills, @iabtcf library or the consuming CMP library. I tried to test it out by updating @iabtcf to accept a Symbol polyfill passed in, but I'm not familiar enough with typescript to make the appropriate change.

nealrosen04 avatar Mar 25 '20 19:03 nealrosen04

or import the polyfilled objects into the file that uses them.

Do the PolyFills not descend down the scope?

For example: if you have your CMP class that you have building with the core-js library

import 'core-js';
import {TCModel} from '@iabtcf/core';

class CMP {
  
  private tcModel: TCModel;

  public constructor() {

    this.tcModel = new TCModel();

  }

}

The @iabtcf/core Library doesn't use the PolyFills and errors on ie 10/11?

chrispaterson avatar Mar 25 '20 23:03 chrispaterson

import 'core-js'; sets the polyfills in the global scope. So yes it would make those polyfills available to the iabtcf code, but it would also pollute the global namespace of the page.

I tried an approach like this and broke some of our sites due to some conflict with the polyfills I was setting and what the page was expecting

nealrosen04 avatar Mar 25 '20 23:03 nealrosen04

A right... As I'm reading a bit further it seems like this might be a good solution...

https://github.com/zloirock/core-js/blob/master/README.md#babelruntime

Are you using babel for your build?

chrispaterson avatar Mar 26 '20 00:03 chrispaterson

Aha, yes I am. This looks like just what we want. I'll try this first thing in the morning and report back. Thanks!

nealrosen04 avatar Mar 26 '20 00:03 nealrosen04

Just an update. It seems like the @babel/runtime approach should work, but I'm having trouble configuring babel to include the polyfills correctly. Would you consider having your build generate both a non-polyfilled and a polyfilled (IE10+) version of lib files?

I'm still trying to get the @babel/runtime approach to work, but just thought I'd float the request and see what your thoughts are on it.

nealrosen04 avatar Mar 26 '20 14:03 nealrosen04

@chrispaterson I'm also trying to add polyfill's to tcf library with @babel/runtime, but facing issues with dependencies from core-js are not bundled in proper order. @nealrosen04 Any luck with @babel/runtime.

mghouse036 avatar Apr 03 '20 14:04 mghouse036

I have not been able to get @babel/runtime to work. It seemed to work to inject polyfills into my own source code, but didn't seem to update anything in this library, possibly because this library code is in my node_modules instead of my src directory. Some online documentation suggested this should still be able to work, but I wasn't able to figure it out before having to put it aside for some other priorities.

nealrosen04 avatar Apr 03 '20 14:04 nealrosen04

@nealrosen04 any updates on this? Is this a viable solution?

chrispaterson avatar Apr 03 '20 17:04 chrispaterson

I posted an update a few hours ago :). I spent a couple days trying to configure babel to inject polyfills into the iabtcf library code, but wasn't able to get it to work. How babel works is very opaque, so I was not having much luck troubleshooting why things weren't working. I had to put this aside and prioritize some other work, so I've disabled the CMP from loading in IE for now.

nealrosen04 avatar Apr 03 '20 17:04 nealrosen04

@nealrosen04 @chrispaterson curious whether anyone was able to get this to work?

HeinzBaumann avatar Apr 15 '20 18:04 HeinzBaumann

Hi @HeinzBaumann, I have not gotten this to work in IE yet. It's been a while since you asked this question. Were you able to get this to work by chance? :). For our CMP, we detect when we're in IE and don't load the library in that case. I don't know what the "right" solution is for enabling IE support here.

nealrosen04 avatar May 21 '20 18:05 nealrosen04

We use the following approach to get past the unit testing on IE 11 with just @iabtcf/core. Essentially babel needs to include @iabtcf modules, and to recognizes them correctly as commonjs. It's not been tried on the other part of the tcf library, but might be worth a try.

     {
        test: /\.js$/,
        exclude: /node_modules(?!\/@iabtcf)/,  // exclude everything in node_modules except @iabtcf
        use: {
          loader: 'babel-loader',
          options: {
            sourceType: 'unambiguous',  // recognize both modules and scripts
            presets: [['@babel/preset-env', {
              useBuiltIns: 'usage',
              corejs: 3
            }]],
          },
        },
      },

pycnvr avatar Jun 08 '20 18:06 pycnvr

This is a webpack module rule that could be used; I agree that would work as a way around the problem. I'd like to be able to provide a better solution for the library. I think the solution is with @babel/plugin-transform-runtime Did you look into that at all @pycnvr?

chrispaterson avatar Jun 08 '20 18:06 chrispaterson

@chrispaterson Yes, I did some experiments with plugin-transform-runtime. My understanding is that there are two approaches. If the tcf libs are built with it, then tcf libs should specify @babel/runtime as a peer dependency. The client code then also add @babel/runtime as a dependency, and there is no need for babel to parse tcf again during client build. The downside is that client must also use plugin-transform-runtime instead of another polyfill such as the ones in preset-env. However, if tcf libs aren't built with plugin-transform-runtime to begin with, then the approach above is still needed so babel can fix up codes.

pycnvr avatar Jun 08 '20 19:06 pycnvr