Skip to content
This repository was archived by the owner on Oct 12, 2021. It is now read-only.

JS: Packaging help #159

Closed
edd opened this issue Mar 25, 2021 · 0 comments · Fixed by #160
Closed

JS: Packaging help #159

edd opened this issue Mar 25, 2021 · 0 comments · Fixed by #160

Comments

@edd
Copy link
Contributor

edd commented Mar 25, 2021

Our current packaging up of the JS is poor in a number of ways. First and foremost: the main script.

api/js/package.json

Lines 5 to 10 in d9e0eda

"main": "bundle/index.js",
"types": "./index.d.ts",
"scripts": {
"build": "npm run lint && npm run build:ts && npm run build:browser",
"build:ts": "tsc",
"build:browser": "browserify -p tinyify index.js > bundle/index.js",

main is set to bundle.index.js for a reason I'll go in to later. However trying to import the browserified bundle in vegaprotocol/console doesn't work. Because it's browserified, which isn't what we need over there.

Our current problem

We're trying to remove our usage of the prepareX endpoints. Previously we had no way of making protos for the wallet server to sign, but this api repo gives us that, so we're doing a spike to try it out

yarn add @vegaprotocol/vega-grpc

Works fine, and

import { vega } from '@vegaprotocol/vega-grpc'
// ...
const order = new vega.OrderSubmission()
order.setPartyId('1234')

In that example above, Intellij / VSCode / etc will correctly show the types and autocomplete. However when we get to actually running it in the browser, it won't bundle (will paste the error in a second)

Importing from '@vegaprotocol/vega-grpc/index' does work though?

Easy! Just remove that main declaration

Yes. This will fix it. The reason that Browserify bundle is there at all is when it comes to signing code using Tweetnacl to sign stuff, we turned out to need Buffer from node. Which leads to other issues (e.g. dchest/tweetnacl-js#59). However we're not using this functionality at the moment - so removing the main declaration is probably the way to go

Related

Also the weird mix of Typescript & JS is a little odd. We should fix that - but that's a separate issue.

edd added a commit that referenced this issue Mar 25, 2021
Closes #159 by stopping setting the bundled, browserified output as the `main` script

- Remove `browserify` dep
- Noop `build:browser`
- Remove `build:browser` from `build`
- Update `main` to `index.js`
@edd edd closed this as completed in #160 Mar 26, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant