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

Add fake chalk in browser builds in order to support IE10 #4367

Merged
merged 6 commits into from
Aug 27, 2017

Conversation

SimenB
Copy link
Member

@SimenB SimenB commented Aug 26, 2017

Summary
Adds a new module jest-fake-chalk which mocks out the entire chalk API in order for expect to work in older browsers.

The implementation is a noop (just returns the string it got as input).

Closes #4074

(First commit is from #4366, so you can merge that and I'll rebase, or just merge this).

/cc @skovhus @mjackson

Test plan
./jest and manually running browser test in IE10

We should really look into getting browserstack or something set up!

image

* @flow
*/

import ansiStyles from 'ansi-styles';
Copy link
Member Author

@SimenB SimenB Aug 26, 2017

Choose a reason for hiding this comment

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

I thought about doing a try-catch around the normal chalk, and just create a fake one if it fails. But I don't think that matters?

@@ -48,6 +48,13 @@ function browserBuild(pkgName, entryPath, destination) {
entry: entryPath,
onwarn: () => {},
plugins: [
{
resolveId(id) {
Copy link
Member Author

Choose a reason for hiding this comment

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

inspired by rollup/rollup#661

@codecov-io
Copy link

codecov-io commented Aug 26, 2017

Codecov Report

Merging #4367 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff           @@
##           master   #4367   +/-   ##
======================================
  Coverage    56.2%   56.2%           
======================================
  Files         191     191           
  Lines        6411    6411           
  Branches        6       6           
======================================
  Hits         3603    3603           
  Misses       2805    2805           
  Partials        3       3

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 39f83a9...df82878. Read the comment docs.

@skovhus
Copy link
Contributor

skovhus commented Aug 26, 2017

As an alternative to this, could we use the previous version of Chalk?

Copy link
Contributor

@skovhus skovhus left a comment

Choose a reason for hiding this comment

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

Does the project have a browserstack or sauce labs account? Would be very nice to add this.

@@ -0,0 +1,14 @@
{
"name": "jest-fake-chalk",
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it make sense to have this living outside the packages folder as it really isn't a package, but a stub used only in the browser build?

Copy link
Member Author

Choose a reason for hiding this comment

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

then it's a bit messy plugging in dependencies, but I have no feelings on the matter

Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@SimenB
Copy link
Member Author

SimenB commented Aug 26, 2017

As an alternative to this, could we use the previous version of Chalk?

I don't think browser support should block updates to modules that doesn't even work in the browser anyways.

Does the project have a browserstack or sauce labs account? Would be very nice to add this.

I don't know...

**/__mocks__/**
**/__tests__/**
src
yarn.lock
Copy link
Member

Choose a reason for hiding this comment

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

rm

Copy link
Member Author

Choose a reason for hiding this comment

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

Done (technically we can remove the whole file since this will not be published)

@cpojer
Copy link
Member

cpojer commented Aug 27, 2017

Nice, thanks for fixing this stuff. I would prefer not to create a separate package for it, and putting packages/expect/src/fake-chalk.js makes sense to me (assuming that we transform that file afterwards?)

@SimenB
Copy link
Member Author

SimenB commented Aug 27, 2017

I can move it to expect, sure.

Can we set up browserstack or something similar?

@cpojer
Copy link
Member

cpojer commented Aug 27, 2017

Could we somehow mock out the environment in Jest? Could that be sufficient for now?

@SimenB
Copy link
Member Author

SimenB commented Aug 27, 2017

For chalk's case in specific it's just Object.setPrototypeOf. I don't think messing up the env really proves support. I only think real browser test provides that value. We can make sure to manually test before release until it's set up?

After moving fake_chalk it's still green, BTW:
image

@SimenB
Copy link
Member Author

SimenB commented Aug 27, 2017

I can confirm that using the expect package built with this diff makes remix-run/history#468 green.

Example of assertion error in the terminal:
image

(Stack trace is different because of #4035)

@skovhus
Copy link
Contributor

skovhus commented Aug 27, 2017

Awesome @SimenB!

I only think real browser test provides that value.

Agree.

@cpojer cpojer merged commit 51e0a24 into jestjs:master Aug 27, 2017
@SimenB SimenB deleted the ie10 branch August 27, 2017 21:01
@SimenB SimenB mentioned this pull request Oct 8, 2020
@github-actions
Copy link

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 13, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

jest-matchers fails on IE10
5 participants