What is has referring to here?

https://github.com/dojo/framework/tree/master/src/has/README.md

thanks!

cross-fetch might be a better option for this (I haven't used either): https://github.com/lquixada/cross-fetch#why-not-isomorphic-fetch

Good point, I've updated this to use cross-fetch

Can we fix this dep please?

I think this causes issues with the bundling on the build side - we get two unnamed bundles (one for the browser and one for node).

Although when building in legacy mode it looks like it's actually bundling correctly in the platform/fetch bundle.

I'm not sure what the best way to resolve this would be then. Seems like what we'd really want is two separate static imports, one with
!has('build-elide') as well as has('host-node') and one for the browser.

But I'm not sure that would behave as expected.

so I think we don't actually ever need to run fetch in node as node is only a supported target environment under test. given thats the complaint (whatwg-fetch blowing up in tests), we can perhaps just use a polyfill that doesn't, and then we don't need to complicate this. For example: https://github.com/developit/unfetch I think doesn't blow up.

Do you think it's possible to wrap all the mocking / replacement parts in a has check for test? At build time this should be then converted to false and then removed from the prod build?

👍 I'll just need to make a change in cli-build-app then to set this flag for the test build.

This seems a bit verbose to have to do for each shim, also I don't think we need to wrap this outside of a test. I wonder if we could just essentially take your wrapping constructor idea and use it generically across the modules, kind of like a "live binding" wrapper to the global? That way we wouldn't even need a replace function per shim, they could just change the global?

function wrapper(nameOnGlobal: string) {
    return function(...args: any[]) {
        return new global[nameOnGlobal](...args);
    };
}

What IntersectionObserver.ts would then look like:

import global from './global';
`!has('build-elide')`;
import 'intersection-observer';
import has from '../has/has';
import wrapper from './somewhere/wrapper';

let value = global.IntersectionObserver;

if (has('test')) {
    value = wrapper('IntersectionObserver') ;
}

export default value as typeof IntersectionObserver;

so I think we don't actually ever need to run fetch in node as node is only a supported target environment under test. given thats the complaint (whatwg-fetch blowing up in tests), we can perhaps just use a polyfill that doesn't, and then we don't need to complicate this. For example: https://github.com/developit/unfetch I think doesn't blow up.

Could we only use the wrapper when the has flag for test is true? This means that when in dist or dev the native implementation will be used (and also prevents the user from being able to change the implementation in anything other than test).

I have the wrapper now returning the original when the test flag is not set to true. So they'll always get the native/polyfilled implementation.

As in, it returns global[nameOfThing] one time instead of a function that provides a live binding.

if we can avoid doing this outside of test that would be ideal, just because otherwise say window.IntersectionObserver is not going to have equality with this return due to the wrapping bind?

The only reason I put this here is because fetch was already doing this. I could just put another parameter on wrapper to control this and make sure it only happens for fetch, or remote that entirely if it wasn't necessary in the first place.

@maier49 cool, perhaps it wasn't needed in the first place?

The bind on fetch was added here: #192

@matt-gadd I can't think of or find any reason why it would be. I've gone ahead and removed it.

@maier49 did you see the original issue as to why fetch was bound?

@agubler Nope, sorry, missed that. I will add another flag to the wrapper then.

Fixed