Use rollup for browser builds #3532

skovhus · 2017-05-09T20:26:23Z

Summary

In order to provide browser versions of relevant Jest packages (#3360) we've tried:

Better browser support #3421 (adding build-es5 infrastructure using babel)
Add babel-transform-runtime to browser builds #3476 (adding babel-transform-runtime)
and now this PR

Here we actually do a proper rollup build including all dependencies and handle packages expecting node globals like process. Similar to what prettier does (https://github.com/prettier/prettier/blob/master/docs/rollup.config.js).

Test plan

I've added an automated test that runs these packages in a browser.

After merging, I would like Jest beta build, so I can test this on the repos I'm trying to convert to Jest + Expect using jest-codemods. skovhus/jest-codemods#39

skovhus · 2017-05-09T20:28:49Z

scripts/build.js

-    if (browser.indexOf(BUILD_ES5_DIR) !== 0) {
-      throw new Error(
-        `browser field for ${pkgJsonPath} should start with "${BUILD_ES5_DIR}"`
+async function buildBrowserPackages(packages) {


Using async/await and for of loops here in order for getting a nice stdout output similar to the node builds.

The current appveyor with node 6 doesn't like this... So should we upgrade the nodejs_version to 7 or should I just get rid of async/await?

async function buildBrowserPackages(packages) { ^^^^^^^^ SyntaxError: Unexpected token function

I just removed it...

codecov-io · 2017-05-09T21:39:38Z

Codecov Report

Merging #3532 into master will decrease coverage by 0.02%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #3532      +/-   ##
==========================================
- Coverage   58.09%   58.07%   -0.03%     
==========================================
  Files         195      195              
  Lines        6751     6747       -4     
  Branches        6        6              
==========================================
- Hits         3922     3918       -4     
  Misses       2826     2826              
  Partials        3        3

Impacted Files	Coverage Δ
packages/jest-matchers/src/to_throw_matchers.js	`77.55% <ø> (-0.45%)`	⬇️
packages/jest-matchers/src/spy_matchers.js	`95.08% <ø> (-0.08%)`	⬇️
packages/jest-matchers/src/matchers.js	`99.37% <ø> (-0.01%)`	⬇️
packages/jest-matchers/src/jasmine_utils.js	`79.62% <ø> (-0.19%)`	⬇️
...matchers/src/extract_expected_assertions_errors.js	`7.14% <ø> (-6.2%)`	⬇️
packages/jest-matchers/src/jest_matchers_object.js	`77.77% <100%> (-2.23%)`	⬇️
packages/jest-matchers/src/utils.js	`96% <100%> (-0.08%)`	⬇️
packages/jest-matchers/src/asymmetric_matchers.js	`90.54% <100%> (+0.39%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c8a5135...37b01bf. Read the comment docs.

SimenB · 2017-05-10T05:15:23Z

Nice! I don't know how much work adding a few browser tests (using mocha, along with jest matchers and jest mocks?) running with karma or something would be? I think that needs to be included in the Jest repo to avoid unknowingly breaking it in the future.

skovhus · 2017-05-10T06:00:13Z

Setting up the tests is rather easy, but we need infrastructure to run the tests on a CI.

skovhus · 2017-05-10T06:03:02Z

For example sauce labs or browser stack. A docker image with a headless browser is also a possibility. Not sure if Jests CI supports that. : )

quantizor · 2017-05-22T03:31:06Z

@skovhus Travis has some recommendations for headless testing that can be done without another third party service: https://docs.travis-ci.com/user/gui-and-headless-browsers/#Using-xvfb-to-Run-Tests-That-Require-a-GUI

skovhus · 2017-06-02T20:47:57Z

@probablyup @SimenB I added some Karma based tests... Works on CI! 👍

Any thoughts/wishes on where to place these tests? And do you want a separate package.json to contain all the karma related things?

Another things: I removed some of the browser/es5 builds that I added earlier. I think it might be enough for now to support jest-mock and jest-matchers.

skovhus · 2017-06-12T13:19:02Z

@SimenB @cpojer let me know if anything is missing here...

skovhus · 2017-06-23T06:11:44Z

@aaronabramov, a few open questions:

anything missing before this can be merged?
any thoughts/wishes on where to place the browser tests? And do you want a separate package.json to contain all the karma related things?

SimenB · 2017-06-23T06:15:12Z

No idea how I lost track of this. Awesome that you were able to add a browser test! Really looking forward to replacing chai and sinon in the browser tests we've got at work.

SimenB · 2017-06-23T06:16:17Z

integration_tests/browser-support/browser-test.js

+
+  it('runs mocks', function() {
+    var someMockFunction = mock.fn();
+    expect(someMockFunction.mock.calls.length).toBe(0);


expect(someMockFunction).not.toHaveBeenCalled();?

cpojer · 2017-06-27T08:30:41Z

Do you mind rebasing this one?

skovhus · 2017-06-27T12:16:09Z

@cpojer sure... I gave it a quick try, but #3778 gives me some issues do to some bable configuration.

Error: 'getMatchers' is not exported by packages/jest-matchers/src/jest_matchers_object.js

Will look more into this at some point. But ideas are more than welcome @SimenB

Also let me know your thoughts on the two questions above (#3532 (comment)).

SimenB · 2017-06-27T13:33:27Z

Hmm, we might have to convert to export as well. Breaking change for programmatic usage though...

Started on this, but it doesn't really scale:

diff --git i/scripts/browserBuild.js w/scripts/browserBuild.js
index e0c6a5ee..570d5630 100644
--- i/scripts/browserBuild.js
+++ w/scripts/browserBuild.js
@@ -47,7 +47,31 @@ function browserBuild(pkgName, entryPath, destination) {
     plugins: [
       rollupFlow(),
       rollupJson(),
-      rollupCommonjs(),
+      rollupCommonjs({
+        namedExports: {
+          'packages/jest-matchers/src/jasmine_utils.js': [
+            'equals',
+            'fnNameFor',
+            'hasProperty',
+            'isA',
+            'isUndefined',
+          ],
+          'packages/jest-matchers/src/jest_matchers_object.js': [
+            'getMatchers',
+            'getState',
+            'setMatchers',
+            'setState',
+          ],
+        },
+      }),
       rollupBabel(babelEs5Options),
       rollupGlobals(),
       rollupBuiltins(),

SimenB · 2017-06-27T13:39:07Z

scripts/build.js

+      p.split('/').pop(),
+      path.resolve(srcDir, 'index.js'),
+      path.resolve(p, browser)
+    ).then(() => {


can you add a catch here?

(node:21659) UnhandledPromiseRejectionWarning: Unhandled promise rejection (rejection id: 1): Error: 'getMatchers' is not exported by packages/jest-matchers/src/jest_matchers_object.js vs

{ Error: 'getMatchers' is not exported by packages/jest-matchers/src/jest_matchers_object.js at error (/Users/simbekkh/repos/jest/node_modules/rollup/dist/rollup.js:170:12) at Module.error$1 [as error] (/Users/simbekkh/repos/jest/node_modules/rollup/dist/rollup.js:8007:2) at Module.trace (/Users/simbekkh/repos/jest/node_modules/rollup/dist/rollup.js:8106:9) at ModuleScope.findDeclaration (/Users/simbekkh/repos/jest/node_modules/rollup/dist/rollup.js:7691:22) at Scope.findDeclaration (/Users/simbekkh/repos/jest/node_modules/rollup/dist/rollup.js:5497:39) at CallExpression.bind (/Users/simbekkh/repos/jest/node_modules/rollup/dist/rollup.js:6022:28) at /Users/simbekkh/repos/jest/node_modules/rollup/dist/rollup.js:5291:50 at VariableDeclarator.eachChild (/Users/simbekkh/repos/jest/node_modules/rollup/dist/rollup.js:5308:5) at VariableDeclarator.bind (/Users/simbekkh/repos/jest/node_modules/rollup/dist/rollup.js:5291:7) at /Users/simbekkh/repos/jest/node_modules/rollup/dist/rollup.js:5291:50 code: 'MISSING_EXPORT', url: 'https://github.com/rollup/rollup/wiki/Troubleshooting#name-is-not-exported-by-module', pos: 1321, loc: { file: '/Users/simbekkh/repos/jest/packages/jest-matchers/src/index.js', line: 31, column: 29 }, frame: '29: import { equals } from \'./jasmine_utils\';\n30: import { any, anything, arrayContaining, objectContaining, stringContaining, stringMatching } from \'./asymmetric_matchers\';\n31: import { getState, setState, getMatchers, se tMatchers } from \'./jest_matchers_object\';\n ^\n32: import extractExpectedAssertionsErrors from \'./extract_expected_assertions_errors\';' }

diff --git i/scripts/build.js w/scripts/build.js index 5f73065c..88d084ce 100644 --- i/scripts/build.js +++ w/scripts/build.js @@ -94,7 +94,11 @@ function buildBrowserPackage(p) { ).then(() => { process.stdout.write(adjustToTerminalWidth(`${path.basename(p)}\n`)); process.stdout.write(`${OK}\n`); - }); + }) + .catch(e => { + console.error(e); + process.exit(1); + }); } }

SimenB · 2017-06-27T13:40:30Z

integration_tests/browser-support/browser-test.js

+    var someMockFunction = mock.fn();
+    expect(someMockFunction.mock.calls.length).toBe(0);
+    someMockFunction();
+    expect(someMockFunction.mock.calls.length).toBe(1);


expect(someMockFunction).toHaveBeenCalled(); or expect(someMockFunction).toHaveBeenCalledTimes(1);

cpojer · 2017-06-27T13:53:15Z

I prefer not to use exports for now as this would be another large codemod. Could we use it only for jest-matcher-utils for now and its dependencies? I'd prefer it if there was a way to make it work with current ES module imports that don't use babel as well – is there such a plugin for babel except the default one?

SimenB · 2017-06-27T13:58:57Z

Could we use it only for jest-matcher-utils for now and its dependencies?

"It" as in export or continue with the whitelisting in the rollup plugins config?

cpojer · 2017-06-27T14:15:41Z

export.

skovhus · 2017-06-27T15:43:24Z

packages/jest-matchers/src/extract_expected_assertions_errors.js

@@ -69,4 +69,4 @@ const extractExpectedAssertionsErrors = () => {
  return result;
 };

-module.exports = extractExpectedAssertionsErrors;
+export default extractExpectedAssertionsErrors;


@cpojer what this what you proposed? : )

skovhus · 2017-06-27T15:47:46Z

@cpojer @SimenB I've added named exports for jest-matchers and addressed the review comments.

cpojer · 2017-06-28T10:08:30Z

Awesome.

cpojer · 2017-06-28T10:16:56Z

Published jest-matchers@20.1.0-alpha.1.

skovhus · 2017-06-28T12:28:01Z

Thanks for merging! Exciting 🌟

Let us try it in the wild!

skovhus · 2017-06-28T12:28:50Z

@cpojer I guess once this is tested and we are happy, then the documentation should be updated.

kentcdodds · 2017-06-28T16:10:35Z

documentation should be updated.

Yes, I'd like to try this out, but I'm not sure where to start 😅

skovhus · 2017-06-28T16:15:11Z

😉

bestander · 2017-07-01T00:57:01Z

@skorvus, I lack some context here, could you help me out?
I rebased workspace PR #3906 on master and now it uses Yarn Workspaces to install node_modules instead of Lerna.

The result is that rollup browser builds don't transpile ES6 constructs correctly

/home/ubuntu/jest/packages/jest-matchers/build-es5/index.js
  4593:1  error  Parsing error: The keyword 'const' is reserved

Any idea why running node ./scripts/build.js would not get buildBrowserPackage to produce the same bundle?

Technically the only difference is that symlinks to other workspaces are not in jest-matchers/node_modules but are in ../../node_modules

skovhus · 2017-07-04T20:11:17Z

Let us discuss this in your PR.

* Remove browser builds for sub dependencies * Add karma browser tests for es5 builds * Use rollup for browser builds * Review comment + prettier * Fix rollup build by using export for jest-matchers * Run prettier on unrelated file

github-actions · 2021-05-13T11:07:11Z

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

facebook-github-bot added the CLA Signed ✔️ label May 9, 2017

skovhus changed the title ~~Use rollup for browser build to include all dependencies~~ Use rollup for browser builds May 9, 2017

skovhus commented May 9, 2017

View reviewed changes

skovhus force-pushed the browser-support-v3 branch 2 times, most recently from 3c45bcc to d3ed28b Compare May 9, 2017 20:43

skovhus force-pushed the browser-support-v3 branch from d3ed28b to c54e4d0 Compare June 2, 2017 19:34

skovhus force-pushed the browser-support-v3 branch from 4268760 to 5861ddb Compare June 2, 2017 20:51

SimenB reviewed Jun 23, 2017

View reviewed changes

skovhus force-pushed the browser-support-v3 branch 3 times, most recently from 503a03d to 49ddf7a Compare June 27, 2017 12:13

SimenB reviewed Jun 27, 2017

View reviewed changes

skovhus added 2 commits June 27, 2017 17:41

Remove browser builds for sub dependencies

597afb7

Add karma browser tests for es5 builds

54462cd

skovhus added 2 commits June 27, 2017 17:41

Use rollup for browser builds

c853943

Review comment + prettier

4f09124

skovhus force-pushed the browser-support-v3 branch from 17179c8 to 7eeced6 Compare June 27, 2017 15:42

skovhus commented Jun 27, 2017

View reviewed changes

Fix rollup build by using export for jest-matchers

411c89e

skovhus force-pushed the browser-support-v3 branch from 7eeced6 to 411c89e Compare June 27, 2017 15:44

Run prettier on unrelated file

37b01bf

SimenB approved these changes Jun 28, 2017

View reviewed changes

cpojer merged commit ac25993 into jestjs:master Jun 28, 2017

skovhus deleted the browser-support-v3 branch June 28, 2017 12:27

pedrottimark mentioned this pull request Jul 10, 2017

Provide ES5 browser build for pretty-format #4002

Closed

This was referenced Jul 19, 2017

jest-matchers fails on IE10 #4074

Closed

Use jest-matchers for assertions instead of expect remix-run/history#468

Merged

github-actions bot locked as resolved and limited conversation to collaborators May 13, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use rollup for browser builds #3532

Use rollup for browser builds #3532

skovhus commented May 9, 2017 •

edited

Loading

skovhus May 9, 2017

skovhus May 9, 2017 •

edited

Loading

skovhus May 9, 2017 •

edited

Loading

codecov-io commented May 9, 2017 •

edited

Loading

SimenB commented May 10, 2017

skovhus commented May 10, 2017

skovhus commented May 10, 2017

quantizor commented May 22, 2017

skovhus commented Jun 2, 2017 •

edited

Loading

skovhus commented Jun 12, 2017

skovhus commented Jun 23, 2017

SimenB commented Jun 23, 2017

SimenB Jun 23, 2017 •

edited

Loading

cpojer commented Jun 27, 2017

skovhus commented Jun 27, 2017

SimenB commented Jun 27, 2017 •

edited

Loading

SimenB Jun 27, 2017

SimenB Jun 27, 2017

cpojer commented Jun 27, 2017

SimenB commented Jun 27, 2017 •

edited

Loading

cpojer commented Jun 27, 2017

skovhus Jun 27, 2017

skovhus commented Jun 27, 2017

cpojer commented Jun 28, 2017

cpojer commented Jun 28, 2017

skovhus commented Jun 28, 2017

skovhus commented Jun 28, 2017

kentcdodds commented Jun 28, 2017

skovhus commented Jun 28, 2017

bestander commented Jul 1, 2017

skovhus commented Jul 4, 2017

github-actions bot commented May 13, 2021

Use rollup for browser builds #3532

Use rollup for browser builds #3532

Conversation

skovhus commented May 9, 2017 • edited Loading

skovhus May 9, 2017

Choose a reason for hiding this comment

skovhus May 9, 2017 • edited Loading

Choose a reason for hiding this comment

skovhus May 9, 2017 • edited Loading

Choose a reason for hiding this comment

codecov-io commented May 9, 2017 • edited Loading

Codecov Report

SimenB commented May 10, 2017

skovhus commented May 10, 2017

skovhus commented May 10, 2017

quantizor commented May 22, 2017

skovhus commented Jun 2, 2017 • edited Loading

skovhus commented Jun 12, 2017

skovhus commented Jun 23, 2017

SimenB commented Jun 23, 2017

SimenB Jun 23, 2017 • edited Loading

Choose a reason for hiding this comment

cpojer commented Jun 27, 2017

skovhus commented Jun 27, 2017

SimenB commented Jun 27, 2017 • edited Loading

SimenB Jun 27, 2017

Choose a reason for hiding this comment

SimenB Jun 27, 2017

Choose a reason for hiding this comment

cpojer commented Jun 27, 2017

SimenB commented Jun 27, 2017 • edited Loading

cpojer commented Jun 27, 2017

skovhus Jun 27, 2017

Choose a reason for hiding this comment

skovhus commented Jun 27, 2017

cpojer commented Jun 28, 2017

cpojer commented Jun 28, 2017

skovhus commented Jun 28, 2017

skovhus commented Jun 28, 2017

kentcdodds commented Jun 28, 2017

skovhus commented Jun 28, 2017

bestander commented Jul 1, 2017

skovhus commented Jul 4, 2017

github-actions bot commented May 13, 2021

skovhus commented May 9, 2017 •

edited

Loading

skovhus May 9, 2017 •

edited

Loading

skovhus May 9, 2017 •

edited

Loading

codecov-io commented May 9, 2017 •

edited

Loading

skovhus commented Jun 2, 2017 •

edited

Loading

SimenB Jun 23, 2017 •

edited

Loading

SimenB commented Jun 27, 2017 •

edited

Loading

SimenB commented Jun 27, 2017 •

edited

Loading