Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

Make parity.js usable by Node and Browser #3475

Merged
merged 8 commits into from
Nov 18, 2016
Merged

Conversation

ngotchac
Copy link
Contributor

Fixes #3471

const Api = require('/path/to/parity/js/.build/parity.js').Api;
const transport = new Api.Transport.Http('http://localhost:8545');
const api = new Api(transport);

api.eth
  .blockNumber()
  .then((block) => {
    console.log('At block', block.toFormat());
    process.exit(0);
  })
  .catch((e) => {
    console.error(e);
    process.exit(1);
  });

@ngotchac ngotchac added A0-pleasereview 🤓 Pull request needs code review. M6-rpcapi 📣 RPC API. labels Nov 16, 2016
@tomusdrw
Copy link
Collaborator

Why not just use const fetch = require('isomorphic-fetch'); instead of relying on global variable to be present?

@jacogr
Copy link
Contributor

jacogr commented Nov 16, 2016

Agreed with @tomusdrw here - just include isomorphic-fetch as part of the entry point (instead of whatwg-fetch) and no other hoops to jump through. (Just webpack lib build needs to include it, which it will)

@jacogr jacogr added A5-grumble 🔥 Pull request has minor issues that must be addressed before merging. and removed A0-pleasereview 🤓 Pull request needs code review. labels Nov 16, 2016
@ngotchac
Copy link
Contributor Author

So I'm not sure how this library is distributed on NPM... ? I had to do this trick because I thought the built JS file had to work in Node and in a browser

@tomusdrw
Copy link
Collaborator

Hmm, so I thought thatisomorphic-fetch is actually taking care of that (choosing right polyfill depending on the environment).
So my understanding was that we should not just "append that to the build", but rather alter the code using fetch to import it explicilty.

@ngotchac
Copy link
Contributor Author

ngotchac commented Nov 16, 2016

Well isomorphic-fetch loads the right module when in a Node environment, but when Webpack builds the library, it will try to get all the necessary code for the Node side of isomorphic-fetch anyway.

The real solution is to have a proper repo, with a build for browser and a build for Node (or as-is for Node in fact)

@ngotchac ngotchac added A3-inprogress ⏳ Pull request is in progress. No review needed at this stage. and removed A5-grumble 🔥 Pull request has minor issues that must be addressed before merging. labels Nov 16, 2016
@ngotchac ngotchac added A0-pleasereview 🤓 Pull request needs code review. and removed A3-inprogress ⏳ Pull request is in progress. No review needed at this stage. labels Nov 16, 2016
@ngotchac
Copy link
Contributor Author

@jacogr @tomusdrw This should be better now. node-fetch is a dependency of the library, and the JS parity library only bundles whatwg-fetch.

@jacogr
Copy link
Contributor

jacogr commented Nov 16, 2016

Ok, as discussed, seems it is the best approach.

I just want to test the library as built in both a Node & Browser environment before giving the ok. (Don't want to go cowboy and have a broken module on NPM. Will do so first thing in the morning.)

@gavofyork
Copy link
Contributor

ci failing?

@jacogr jacogr added A3-inprogress ⏳ Pull request is in progress. No review needed at this stage. and removed A0-pleasereview 🤓 Pull request needs code review. labels Nov 17, 2016
@jacogr
Copy link
Contributor

jacogr commented Nov 17, 2016

@ngotchac Just added the in-progress label since there are issues you are working on in here.

@ngotchac ngotchac added A0-pleasereview 🤓 Pull request needs code review. and removed A3-inprogress ⏳ Pull request is in progress. No review needed at this stage. labels Nov 17, 2016
@ngotchac
Copy link
Contributor Author

@jacogr Ok it should be fine now.

You can try it with npm run test:npm for Node integration. In the browser, the window.Parity object is created with Api and Abi.

entry: 'library.js',
output: {
path: path.join(__dirname, '.npmjs'),
filename: 'library.js',
libraryTarget: 'commonjs'
library: 'Parity',
Copy link
Contributor

@jacogr jacogr Nov 17, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/Parity/parity/, need this to work -

const { Api } = window.parity;

const api = new Api(...);

@jacogr jacogr added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Nov 17, 2016
@gavofyork gavofyork merged commit d0cbbe2 into master Nov 18, 2016
@gavofyork gavofyork deleted the ng-fix-parity-jsapi branch November 18, 2016 11:17
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A8-looksgood 🦄 Pull request is reviewed well. M6-rpcapi 📣 RPC API.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants