alpacahq / alpaca-ts

A TypeScript Node.js library for the https://alpaca.markets REST API and WebSocket streams.
ISC License
156 stars 41 forks source link

added multiple bundles output via rollup and fix for commonjs #44

Closed husayt closed 3 years ago

husayt commented 3 years ago

I did go bit wild here. Added bundling via rollup and fix for commonjs. Let's review, please.

husayt commented 3 years ago

Also replaces events with much faster eventemitter3.

117 commented 3 years ago

Reviewing.

117 commented 3 years ago

Getting Error: Cannot find module './client'. Because require("./client") does not have the file extension .cjs. The obvious fix is to just go back to adding the trailing extensions then remapping them. But is there a better way?

husayt commented 3 years ago

It was kind of 3am, but I wanted to share it even if it was bit raw, so we could discuss it. The problem for me I don't have cjs code to test it. Do you mind sharing something even if simple just to see that error. All the unit tests pass, that means we need to add more tests to cover cjs and flag that error.

Along with cjs and mjs, will be good to have a browser bundle (preferably umd as well as esm) , that is why I did add few more bundle options so we could review them and leave what makes most sense.

I will try to add some more examples. Just to show multiple options of using this lib.

husayt commented 3 years ago

Getting Error: Cannot find module './client'. Because require("./client") does not have the file extension .cjs. The obvious fix is to just go back to adding the trailing extensions then remapping them. But is there a better way?

Do we really need cjs extension. Generally, if it is not mjs, then it's cjs. If we just leave commonjs code with js extension, then it will work ok. This is the simplest thing to do here

husayt commented 3 years ago

@117 Have a try now, pls

117 commented 3 years ago

Trying.

117 commented 3 years ago

Still can't get it to import via require, same error.

husayt commented 3 years ago

Is there a sample, where I could test it? I am using this lib from multiple places and all works fine for me.

117 commented 3 years ago

I am just doing node index.js on the example after installing npm i husayt/alpaca.

husayt commented 3 years ago

I have found the problem. Will try to get the solution which allows for node and browser, esm and commonjs consumption

husayt commented 3 years ago

@117 with further tweaking and replacing ky-universal with node-fetch, I got both commonjs and ESM working.

But then, the reason ky-universal and others are stopping commonjs support, is that with node 14 properly supporting ES modules, there is no much point for CJS moving forward. So maybe, we should make the switch too? Unless, we want to support legacy node, there is no need for CJS anymore.

We can just produce two outputs, one for ESM modules and the other with all the dependencies bunlded for browsers. I have already spent two days on this and could get it all done and polished, so we don't need to come back to this anytime soon.

Then we will release the next version with the new major to not to break others who still want to use commonjs.

What do you think?

117 commented 3 years ago

First of all thank you so much for the contributions you have been making! I have been very busy with work and I appreciate you taking the time to improve the library. 🙂

I am all for ditching commonjs support, it has gone the way of dinosaurs and I don't enjoy maintaining it. This does mean we have to push a major version change though.

husayt commented 3 years ago

@117 I have finally got the working configuration. Here I have set main ver to 4.0.0. As we have discussed we are not supporting commonjs going forward. That makes our lives much easier. Instead we have whole set of bundles for different use cases. As a cherry on the cake, I added a microtader demo app which shows how to list and order directly from browser.

There are few changes and updates which I mention below. Hope it all works fine:

To help with tree-shaking there is no default export and import is as following:

import { AlpacaClient, AlpacaStream } from '@master-chief/alpaca'

Use directly from browser

In a browser:

<script src="alpaca.browser.min.js"></script>

Also modern browsers allow

<script type="module">
import quranMeta from "alpaca.esm.min.js"
</script>

The library is available from various CDNs

Distributions and Downloads

Here you can find the following

Source code in typescript TS
Javascript code autotranspiled from TS as ES Next ES
ESM Bundle ES bundled in one file
distributions bundled with dependencies of library as
UMD/ UMD minified builds can be used directly in the browser via a <script> (see here about UMD format) ES+UMD (for classic import user)
ESM Browser /ESM Browser minified ES5+ESM (for modern import type="module")
husayt commented 3 years ago

@117 thanks for reviewing and polishing. You did also mention deno compatibility. I didn't have a chance to test it, but if you manage to try it should work. Will be good to know

117 commented 3 years ago

@husayt So i've gotten it to work with deno!

import { AlpacaClient } from 'https://cdn.skypack.dev/@master-chief/alpaca'

Using the SkyPack CDN, Deno has no problem finding the types.

husayt commented 3 years ago

Great, will be very useful to mention that in readme too.

On Sun, 31 Jan 2021, 01:05 Master Chief, notifications@github.com wrote:

@husayt https://github.com/husayt So i've gotten it to work with deno!

import { AlpacaClient } from 'https://cdn.skypack.dev/@master-chief/alpaca'

Using the SkyPack CDN, Deno has no problem finding the types.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/117/alpaca/pull/44#issuecomment-770307404, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAI5PLA5E7OMO6HLJTUS3ZTS4SUEJANCNFSM4WWLIHWQ .