Skip to content
This repository has been archived by the owner on Aug 9, 2021. It is now read-only.

Js generators break dApps #612

Open
mqklin opened this issue Oct 1, 2019 · 12 comments
Open

Js generators break dApps #612

mqklin opened this issue Oct 1, 2019 · 12 comments
Assignees
Labels
bug Something isn't working documentation
Milestone

Comments

@mqklin
Copy link

mqklin commented Oct 1, 2019

Describe the bug
Projects that use 3box (like https://beta.zerion.io) fails in browsers like Edge or Safari (iPad)

To Reproduce

  1. Go to https://beta.zerion.io on Edge or iPad/Safari
  2. See error in console

Expected behavior
Site should be open

Desktop (checked on browserstack):

  • Device: Windows 10
  • Browser: Edge 18 (Latest)

Tablet:

  • Device: iPad
  • OS: iOS 11.3
  • Browser: Safari

Additional context
After a quick search I found that ipfs uses generators, so maybe you should replace it with another library.

@oed
Copy link
Member

oed commented Oct 1, 2019

Hey @mqklin Thanks for reporting. Can you please provide more information about the error you're seeing. I don't have an ipad or Edge browser to test with.

@oed
Copy link
Member

oed commented Oct 1, 2019

It would not be very easy to replace the ipfs depencency since most of our architecture relies on that. Depending on your situation though there might be good ways around this :)

@mqklin
Copy link
Author

mqklin commented Oct 1, 2019

Here is the error (Syntax error points to generator function *, parser cannot parse asterisk):
image

Maybe we can transpile this dependency on our side using webpack and babel, but AFAIK this is not a good practise, cause all public libs should support some list of the most popular browsers. I believe Edge is quite popular

@oed
Copy link
Member

oed commented Oct 1, 2019

I believe we are transpiling our library to es5, but it seems like this is not sufficient in your case. Do you have any other suggestions for how this could potentially be fixed?

Which features of 3box-js are you currently using in your dapp btw?

@mqklin
Copy link
Author

mqklin commented Oct 2, 2019

We use only getVerifiedAccounts and getProfile. Maybe this should be fixed on ipfs side, I'll create an issue there

@oed
Copy link
Member

oed commented Oct 2, 2019

Actually, if those are the only two functions you are using you could use the dist/3box.api.min.js which doesn't include ipfs! it's also a much smaller dependency.
Sorry that it's not well documented, we should make that much clearer!

If you end up making an issue in the ipfs repo, let me know so I can track it :)

@oed oed added bug Something isn't working documentation labels Oct 2, 2019
@mqklin
Copy link
Author

mqklin commented Oct 3, 2019

We've switched to dist/3box.api.min.js and it works now, thanks!
But maybe you could add this API as separate modules, so we would be able to import only what we need (like lodash do):

import {getVerifiedACcounts, getProfile} from '3box';

Also we are trying to keep all vendors in node_modules, so maybe you could deploy 3box API like a separate npm module, not like a <script>?

@oed
Copy link
Member

oed commented Oct 3, 2019

Hey sorry @mqklin I forgot to mention. You can import it like described here: https://github.com/3box/3box-js#optimize-build-for-read-only-3box-api

@mqklin
Copy link
Author

mqklin commented Oct 3, 2019

I've included 3box in my build and after some research noticed these pieces of code:

node_modules/3box/dist/3box.min.js (147:25398):

...n("Not writing last bytes from original file")}return{[Symbol.asyncIterator]:async function*(){}}})...

node_modules/ipfs/dist/index.min.js (1:2111002):

...p("Not writing last bytes from original file")}return o({},Symbol.asyncIterator,f(i.default.mark(function e(){return i.default.wrap(function(e){for(;;)...

node_modules/ipfs-mfs/dist/index.min.js (1:772919):

...d("Not writing last bytes from original file")}return o({},Symbol.asyncIterator,f(i.default.mark(function e(){return i.default.wrap(function(e){for(;;)...

If I'm not mistaken, they point to the same part of code. As you can see, only 3box.min.js has generator function*, but ipfs transpiles it. So maybe you import it somehow incorrectly? Not sure though

@mqklin
Copy link
Author

mqklin commented Oct 3, 2019

You can import it like described here:

Thanks, I had to read the docs more carefully!

I've just tried this code: import {getProfile, getVerifiedAccounts} from '3box/lib/api';, but after compiling my build still has function* generators, so I'll stick with <script src="https://unpkg.com/3box/dist/3box.api.min.js"></script> for now.

Also it would be super cool if you had a babel-plugin for tree-shaking like lodash has, so user will type import {getProfile, getVerifiedAccounts} from '3box'; and get only what he needs.

@oed
Copy link
Member

oed commented Oct 3, 2019

Ok, that seems like a bug. It should do tree shaking. @zachferland would you mind having a look at this?

@oed
Copy link
Member

oed commented Oct 3, 2019

Sorry missed that you made two comments.

If I'm not mistaken, they point to the same part of code. As you can see, only 3box.min.js has generator function*, but ipfs transpiles it. So maybe you import it somehow incorrectly? Not sure though

Thanks for finding this! Will definitely have to look into and fix this!

@michaelsena michaelsena added this to the Sprint 30 milestone Oct 16, 2019
@oed oed self-assigned this Oct 16, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working documentation
Projects
None yet
Development

No branches or pull requests

3 participants