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

fix(): export setEnv for handling env edge cases #8888

Merged
merged 3 commits into from May 6, 2023
Merged

Conversation

ShaMan123
Copy link
Contributor

@ShaMan123 ShaMan123 commented May 6, 2023

Motivation

Working on an app using CRA (create-react-app, complete 💩 and abandoned as well) that uses jest for testing.
After migrating to v6 got an error related to jsdom, CRA is to blame (can't reproduce with a simple jest example, see below).
What happened is that for some reason the CRA jest config now imports the node entry point whereas before (v5) fabric thought it was running in the browser because of jest mocking window/document.
Seems to be a jsdom mismatch of versions. Didn't look into it. And is an edge case we should not cater for.
Setting env fixes it because it exposes the mocked window/document to fabric.

   TypeError: Failed to execute 'appendChild' on 'Node': parameter 1 is not of type 'Node'.

Description

expose setEnv with a clear comment on usage.

Changes

Gist

If you want to setup jest in order to test your fabric project (in node or in a mocked browser environment) follow this:
Testing fabric with jest

If for some sad reason you are using CRA or encountering the error above then try the following:

import * as fabric from "fabric";
fabric.setEnv({ ...fabric.getEnv(), window, document });

If you wish to mock fabric (no real reason to do that) you can add these lines to your mocked module __mocks__/fabric.ts

In Action

@github-actions
Copy link
Contributor

github-actions bot commented May 6, 2023

Build Stats

file / KB (diff) bundled minified
fabric 903.752 (+0.637) 305.339 (+0.036)

@ShaMan123 ShaMan123 requested a review from asturur May 6, 2023 10:29
@github-actions
Copy link
Contributor

github-actions bot commented May 6, 2023

Coverage after merging export-setEnv into master will be

83.74%

Coverage Report
FileStmtsBranchesFuncsLinesUncovered Lines
index.node.ts7.69%100%0%14.29%17, 20, 23, 29, 32, 35
src
   ClassRegistry.ts100%100%100%100%
   Collection.ts94.71%94.64%86.67%97.09%101, 104, 207–208, 233–234
   CommonMethods.ts96.55%87.50%100%100%10
   Intersection.ts100%100%100%100%
   Observable.ts87.23%85.29%84.62%89.36%144–145, 170–171, 39–40, 48, 57, 91, 99
   Pattern.ts92.21%91.89%90%93.33%114, 138, 149, 158, 51
   Point.ts100%100%100%100%
   Shadow.ts98.36%95.65%100%100%178
   cache.ts97.06%90%100%100%57
   config.ts75%66.67%66.67%82.76%130, 138, 138, 138, 138, 138–140, 151–153
   constants.ts100%100%100%100%
   typedefs.ts100%100%100%100%
src/brushes
   BaseBrush.ts100%100%100%100%
   CircleBrush.ts0%0%0%0%100, 102–104, 113, 113, 113, 115, 117, 119–121, 123–126, 133–134, 141, 143, 28–29, 37–41, 45–49, 56–59, 67–71, 73, 81, 81, 81, 81, 81–82, 84, 84, 84–87, 89, 97–98
   PatternBrush.ts97.06%87.50%100%100%21
   PencilBrush.ts91.01%82.35%100%93.75%122–123, 152, 152–154, 176, 176, 276, 280, 285–286, 68–69, 84–85
   SprayBrush.ts0%0%0%0%100–101, 103–104, 112, 112, 112, 112, 112–113, 115–116, 123–124, 126, 128–132, 141, 145–146, 146, 154, 154, 154–157, 159–162, 166–167, 169, 171–174, 177, 184–185, 187, 189–190, 192, 199–200, 202–203, 206, 206, 213, 213, 217, 22–23, 25–27, 27, 27–29, 33, 42, 49, 56, 63, 70, 89–91, 99
src/canvas
   Canvas.ts78.98%77.54%81.67%79.60%1000–1001, 1001, 1001–1003, 1005–1006, 1006, 1006, 1008, 1016, 1016, 1016–1018, 1018, 1018, 1024–1025, 1033–1034, 1034, 1034–1035, 1040, 1042, 1073–1075, 1078–1079, 1083–1084, 1197–1199, 1202–1203, 1276, 1395, 1518, 1588, 162, 187, 296–297, 300–304, 309, 332–333, 338–343, 363, 363, 363–364, 364, 364–365, 37, 373, 378–379, 379, 379–380, 382, 391, 397–398, 398, 398, 41, 441, 449, 453, 453, 453–454, 456, 538–539, 539, 539–540, 546, 546, 546–548, 568, 570, 570, 570–571, 571, 571, 574, 574, 574–575, 578, 587–588, 590–591, 593, 593–594, 596–597, 609–610, 610, 610–611, 613–617, 623, 630, 670, 670, 670, 672, 674–678, 684, 690, 690, 690–691, 693, 696, 701, 714, 741, 741, 799–800, 800, 800–801, 803, 806–807, 807, 807–808, 810–811, 814, 814–816, 819–820, 890, 902, 909, 930, 962, 983–984
   SelectableCanvas.ts94.39%91.16%94.55%96.63%1112, 1112–1113, 1116, 1136, 1136, 1194, 1247–1248, 1269, 1277, 1402, 1404, 1406–1407, 511, 691–692, 694–695, 695, 695, 744–745, 806–807, 860–862, 894, 899–900, 927–928
   StaticCanvas.ts96.01%91.48%100%97.93%1128–1129, 1129, 1129–1130, 1250, 1260, 1314–1315, 1318, 1353–1354, 1431, 1440, 1440, 1444, 1444, 1491–1492, 1706–1707, 327–328, 345, 425–426, 428–429, 790, 802–803, 888
   TextEditingManager.ts100%100%100%100%
src/color
   Color.ts92.16%86.49%100%94.29%330–331, 335–336, 339–340, 58, 88–89, 89, 91, 91, 91–92, 94–95
   color_map.ts100%100%100%100%
   constants.ts100%100%100%100%
   util.ts100%100%100%100%
src/controls
   Control.ts93.90%88.89%90.91%97.73%235, 322, 322, 357
   changeWidth.ts100%100%100%100%
   commonControls.ts100%100%100%100%
   controlRendering.ts81.63%78%100%84.78%106, 111, 121, 121, 45, 50, 61, 61, 65–72, 81–82
   drag.ts100%100%100%100%
   polyControl.ts5.97%0%0%11.11%100, 105, 119, 119, 119, 119, 119, 121–124, 124, 127, 134, 17, 25–29, 29, 29, 29, 29, 29, 29, 29, 50–56, 56, 56, 56, 56, 58, 63–64, 66, 76, 82–83, 83, 83–84, 88–90, 90, 90, 90, 90, 92
   rotate.ts19.57%12.50%50%21.43%41, 45, 51, 51, 51–52, 55–57, 59, 59, 59, 59, 59–61, 61, 61–63, 65, 65, 65–67, 67, 67–68, 73, 73, 73–74, 76, 78, 80–81
   scale.ts93.57%92.94%100%93.67%129–130, 132–134, 148–149, 181–183, 42
   scaleSkew.ts78.79%64.29%100%85.71%27, 29, 29, 29, 31, 33, 35
   skew.ts91.03%79.31%100%97.67%130–131, 162–163, 170, 176, 178
   util.ts100%100%100%100%
   wrapWithFireEvent.ts100%100%100%100%
   wrapWithFixedAnchor.ts100%100%100%100%
src/env
   browser.ts84.21%77.78%50%100%14, 17
   index.ts100%100%100%100%
   node.ts76.92%50%66.67%88.24%27, 31–32, 32, 32
src/filters
   BaseFilter.ts21.93%22.81%32%19.05%106–109, 109, 109–110, 116, 116, 116–119, 137, 155, 169–174, 178–179, 179, 179–182, 182, 182,

@asturur asturur changed the title bundle(): export setEnv fix(): export setEnv for Jest interoperability May 6, 2023
@asturur asturur merged commit 95b319c into master May 6, 2023
18 checks passed
@asturur
Copy link
Member

asturur commented May 6, 2023

Merged and now i ll push beta5.
@ShaMan123 please can you change this PR description adding an example a test and a setup that was failing, and how did you changed it. I want people eventually that search for JEST in this repo to find this out.

@asturur asturur deleted the export-setEnv branch May 6, 2023 18:07
@ShaMan123 ShaMan123 changed the title fix(): export setEnv for Jest interoperability fix(): export setEnv for handling env edge cases May 7, 2023
@ShaMan123
Copy link
Contributor Author

@asturur apparently this issue wasn't related to jest, at least not to the latest version.
Updated description, you can see under Gist the example app.

@ShaMan123
Copy link
Contributor Author

@asturur apparently this issue wasn't related to jest, at least not to the latest version. Updated description, you can see under Gist the example app.

It proves without doubt that it is possible to use fabric with jest (>=29), importing behavior works as expected.
Take a look at src/snapshots.
Proving this is a problem with older versions of jest or CRA. In which case I don't mean to bother about it.

@ShaMan123
Copy link
Contributor Author

In the website I can make this into a guide about using fabric and jest for tests.

@asturur asturur mentioned this pull request May 19, 2023
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

2 participants