Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] Run tests in CI #45

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from
Draft

[WIP] Run tests in CI #45

wants to merge 4 commits into from

Conversation

lawrence-forooghian
Copy link

No description provided.

We expose an ES module version of the library for module bundlers to
use, and generate a CommonJS version for Node.

The motivation is that, for [1] I want to re-export this library from
ably-js as a tree-shakable module. Tree-shakability requires that we use
ES modules.

I’ve output the Node bundle to a new `build` directory, since I don’t
know whether the `dist` directory has some special meaning for the
`ably-env deploy` script and so didn’t want to put non-CDN stuff in
there.

The version of Node that the README tells us to use does not support ES
modules, so we generate a CommonJS bundle for the tests to use, which
includes all of the internal components so that they can be tested.

I tested these changes for regressions by:

1. running `npm run grunt -- test:node`
2. running `npm run grunt -- test:browser:local`
3. generating an NPM package using `npm pack`, installing it in ably-js,
   and running the delta tests there

[1] ably/ably-js#1492
Motivation as in 9d8aeb8, and tested in the same way.

I don’t know exactly _what_ about the non-class way of doing things made
it not be tree-shakable; I can only speculate that it was the
`.prototype.foo = ...` statements.

The update to Babel is because the version we were using didn’t support
class fields.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant