krakenjs / zoid

Cross domain components
Apache License 2.0
2.01k stars 248 forks source link

Able to choose between component/xcomponent.frame when importing #152

Open r0stig opened 6 years ago

r0stig commented 6 years ago

Hello,

We're using xcomponent (which is an awesome library, good work!) in a TypeScript/webpack project which means we use npm to install xcomponent and then import it: import * as xcomponent from 'xcomponent'

The problem is we want to use the xcomponent without the ie-bridge since it causes errors in IE11 (somehow it thinks we want to use the bridge even though we don't use popups). I don't think we need to dig in this bug since without the IE-specific bridge code it works. If we look how react solves this (dev build vs prod build) they use environment variables to be able to choose which build should be included: https://github.com/facebook/react/blob/master/packages/react/npm/index.js

I was thinking if this might be a good idea for xcomponent as well? Maybe introduce a environment variable and let the user choose if one wan't to import the whole thing or without iframe support?

bluepnume commented 6 years ago

it causes errors in IE11 (somehow it thinks we want to use the bridge even though we don't use popups)

Weird. Can you share some more details about this? I'd like to figure out a solve either way.

Maybe introduce a environment variable and let the user choose if one wan't to import the whole thing or without iframe support?

I'm definitely open to this idea, and if you'd like to PR it in, I'd be more than happy with that. We're already using feature flags in src/ but I'd be more than happy to have them to help decide which bundle to use from dist/

r0stig commented 6 years ago

Weird. Can you share some more details about this? I'd like to figure out a solve either way.

I'll try to explain from memory, I don't have a minimal code to recreate the error.

We're having two iframes in the parent page which needs to communicate between each other. We solve this by having both iframes provide a callback to the parent and have our own kind of bridge to tunnel the messages from one iframe to the other via the provided callbacks. When enabling debug mode we see this error:

Can only use bridge to communicate between two different windows, not between frames.

This error is thrown in the post-robot code: https://github.com/krakenjs/post-robot/blob/c54c30635b4c715c7dffab7ca2371e4ff7c1436c/src/drivers/send/strategies.js#L75 So the question is why is postrobot assuming we use a bridge here? No external window is created. We just have our two iframes.

I'm definitely open to this idea, and if you'd like to PR it in, I'd be more than happy with that. We're already using feature flags in src/ but I'd be more than happy to have them to help decide which bundle to use from dist/

I'll submit a PR.

bluepnume commented 6 years ago

Thanks for the PR! And appreciate you sharing details on the issue. I'll take a look.

bluepnume commented 6 years ago

Oh wait, you only see the issue during debug mode? That's probably expected. The throw there is just used as a way to short-circuit that messaging path. Unless it causes any issues beyond error logs?

Anyway, either way, probably best to use the .frame.js build 👍

r0stig commented 6 years ago

Oh wait, you only see the issue during debug mode? That's probably expected. The throw there is just used as a way to short-circuit that messaging path. Unless it causes any issues beyond error logs?

Yes, the original issue was that message passing between child page and parent page was slow, around 200-300 ms. When enabling debug logs (defaultLogLevel: 'debug') we got the specified error. defaultEnv was still in prod mode. We also got this log in Sentry:

Error: No handler found for post message ack for message: postrobot_method from http://domain1 in https://otherdomain

Note we're having http/https, might that cause this issue?

I'm having a hard time knowing which logs and errors are related to the problem we had. So I didn't know what info to provide in the original issue. Bear with me :-) .

With the .frame.js-build no error occured in the logs and the message passing was super fast. So I'm not sure the error was expected? :-)

doublemarked commented 6 years ago

Hey, I'm using Webpack as well, and want to mention that PR #153 has doubled my webpack build size, presumably because, due to the use of a runtime var, Webpack cannot suss out whether both xcomponent.frame.js and xcomponent.js are required and ends up bundling them both in. To be honest I'm still figuring out XComponent and don't have a clear idea of the difference of these two modules, but I do see my bundle size double after xcomponent@6.0.52.

To mitigate this, I'm just importing directly the frame js (as I also only need the frame version): import xcomponent from 'xcomponent/dist/xcomponent.frame', which imho seems a bit cleaner and explicit than using env-vars.

bluepnume commented 6 years ago

Yikes. Out of interest, which version of webpack are you using, and do you have uglify enabled?

doublemarked commented 6 years ago

Webpack 3, and experienced both with and without uglify enabled.

The specific webpack settings I'm using are those generated by the VueJs CLI 3 boilerplate.

n00b2pr0 commented 6 years ago

I'm using webpack as well and recently changed the xcomponent dependency to zoid. I saw the build size more than double, a similar experience as @doublemarked. I'm still debugging, but found myself on this issue and wanted to note my findings.

xcomponent 6.0.49 was the previous install version.

doublemarked commented 5 years ago

Just upgraded to zoid@9 and still facing this. It seems that changes since zoid@7 now prevent an import of zoid/dist/zoid.frame, but I've got it working by changing to require: require('zoid/dist/zoid.frame')...

Still feels kind of hacky and it's unclear whether this is the intended use of the module.

KyorCode commented 5 years ago

This is still a problem. Even require('zoid/dist/zoid.frame') isn't working in zoid@9.

doublemarked commented 5 years ago

@KyorCode I can't remember the reason, but we are using currently require('zoid/dist/zoid.frameworks')

jimmyn commented 4 years ago

I'm having the same issue.

This does not work, zoid is undefined

import zoid from 'zoid/dist/zoid.frameworks';

This does not work either

import * as zoid from 'zoid/dist/zoid.frameworks';

This somehow works:

const zoid = require('zoid/dist/zoid.frameworks');

I'm using "zoid": "9.0.42"