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

[Bug]: FabricJS does not work with NextJS 13 and the new appDir #8444

Closed
7 tasks done
HMilbradt opened this issue Nov 17, 2022 · 21 comments
Closed
7 tasks done

[Bug]: FabricJS does not work with NextJS 13 and the new appDir #8444

HMilbradt opened this issue Nov 17, 2022 · 21 comments

Comments

@HMilbradt
Copy link

CheckList

  • I agree to follow this project's Code of Conduct
  • I have read and followed the Contributing Guide
  • I have read and followed the Issue Tracker Guide
  • I have searched and referenced existing issues and discussions
  • I am filing a BUG report.
  • I have managed to reproduce the bug after upgrading to the latest version
  • I have created an accurate and minimal reproduction

Version

5.2.4

In What environments are you experiencing the problem?

Node.js

Node Version (if applicable)

16.14.2

Link To Reproduction

https://github.com/HMilbradt/next-13-fabric

Steps To Reproduce

  1. yarn install
  2. yarn dev
  3. http://localhost:3000

Note that I've just used the Next13 playground example. The only relevant files are /app/page.tsx and /app/dynamic-fabric.tsx.

Completely removing the import for /app/dynamic-fabric.tsx will allow the app to run properly.

Expected Behavior

The app should be able to load, if not normally then at least using the NextJS dynamic component with SSR turned off.

Actual Behavior

The app refuses to load, as server rendering fails due to the Node canvas files being loaded.

I've opened a report on NextJS as well here vercel/next.js#43050. Not sure if this issue is entirely on that framework, or it's now an issue that needs to be addressed in Fabric.

For the time being, I will likely be forking fabric and removing the NodeJS code.

Error Message & Stack Trace

error - ./node_modules/canvas/build/Release/canvas.node
Module parse failed: Unexpected character '' (1:0)
You may need an appropriate loader to handle this file type, currently no loaders are configured to process this file. See https://webpack.js.org/concepts#loaders
(Source code omitted for this binary file)
@ShaMan123
Copy link
Contributor

ShaMan123 commented Nov 18, 2022

#8208
duplicate #8258

If you think I should reopen please comment

@ShaMan123
Copy link
Contributor

specifically 30f5a04

@ShaMan123 ShaMan123 added v6 and removed v6 labels Nov 18, 2022
@asturur
Copy link
Member

asturur commented Nov 20, 2022

@HMilbradt to avoid those issues fabric gets published in 2 versions

image

https://www.npmjs.com/package/fabric?activeTab=versions

Please try the version 5.2.4-browser you shouldn't have any node-canvas code.

Alternatively you have to stub out the node dependencies, but i have an example for webpack and i m not sure the latest next.js what is using.

@lmachens
Copy link

@asturur

I tried 5.2.4-browser, but this leads to another error:

./node_modules/fabric/dist/fabric.js:24:0
Module not found: Can't resolve 'jsdom'

Dynamically importing fabric or the component doesn't help too.
Do you have another idea?

@ShaMan123
Copy link
Contributor

ShaMan123 commented Dec 19, 2022

What I suggested works in next 12
I didn't try next 13,
https://beta.nextjs.org/docs/rendering/server-and-client-components#keeping-server-only-code-out-of-client-components-poisoning
https://beta.nextjs.org/docs/rendering/server-and-client-components#convention

Next stuff is enough to resolve this issue
use client seems to be the solution

@lmachens
Copy link

lmachens commented Dec 19, 2022

@ShaMan123

Thx, I was trying out several things (except patching fabric.js).

use client is already added. I tried to dynamically import fabric, but I have the same issue like in:
vercel/next.js#43050
By the way, use client means it is run on server and client, not only server.

Same issues with serverComponentsExternalPackages .

Still no luck.

@ShaMan123
Copy link
Contributor

Using dynamic in next 12 works, but it must import a component so that is annoying but doable

@lmachens
Copy link

This discussion is helpful:
vercel/next.js#42319

But the main difference between fabric.js and other libraries like leaflet is, that fabric assumes it can be running on Node:

var jsdom = require('jsdom');

The server is trying to bundle/pack fabric.js, but thinks it's required to bundle jsdom/canvas too.
This was changed in Next 13.

I think the best solution is to remove the Node related code for the -browser releases.

@ShaMan123
Copy link
Contributor

That is inaccurate.
At the moment fabric guesses the environment once imported.
And in ssr it fails because it doesn't guess at the right time.
We are going to address this in v6. And stop guessing altogether hopefully.

@lmachens
Copy link

I patched the HEADER.js and removed the else. This solves my issue, because it allows me to import fabric in my components, without the need of dynamic import or similar.
And I am already using -browser tag.

fabric.js/HEADER.js

Lines 32 to 52 in 391eb99

} else {
// assume we're running under node.js when document/window are not present
var jsdom = require('jsdom');
var virtualWindow = new jsdom.JSDOM(
decodeURIComponent(
'%3C!DOCTYPE%20html%3E%3Chtml%3E%3Chead%3E%3C%2Fhead%3E%3Cbody%3E%3C%2Fbody%3E%3C%2Fhtml%3E'
),
{
features: {
FetchExternalResources: ['img'],
},
resources: 'usable',
}
).window;
fabric.document = virtualWindow.document;
fabric.jsdomImplForWrapper =
require('jsdom/lib/jsdom/living/generated/utils').implForWrapper;
fabric.nodeCanvas = require('jsdom/lib/jsdom/utils').Canvas;
fabric.window = virtualWindow;
global.DOMParser = fabric.window.DOMParser;
}

Maybe this helps others until you address this in v6.
Thx and good job! Looking forward to test it.

@ShaMan123
Copy link
Contributor

I don't know how you use fabric but I would expect you'd encounter fabric.window, fabric.document is undefined errors

@lmachens
Copy link

No it's fine. After rendering my React component , I initialize it, which is only run on client side.

@ShaMan123
Copy link
Contributor

Awesome.
So it is the next bundling logic perhaps

@ShaMan123
Copy link
Contributor

Update
#8632 should fix SSR out of the box

@ZhengRui
Copy link

ZhengRui commented Feb 24, 2023

Is there a way to get fabricjs v5 work with Nextjs 13, without using dynamic import? I am able to get it work using dynamic import but it increased the loading time significantly and caused other issues. I tried to use fabric@5.3.0-browser, but it has Can not resolve 'canvas' issue. I tried to remove else part in HEADER.js and rebuild fabric as mentioned above, but it didn't help.

Really hope there could be a clarification on this issue, I've been banging my head on this issue for two weeks. And none of discussions about this issue provides a clear solution.

@ShaMan123
Copy link
Contributor

ShaMan123 commented Feb 24, 2023

We made v6 SSR resilient.
V5 isn't.
If you must work on v5 look at the SSR support pr and follow it.
#8632 #8652

@ZhengRui
Copy link

thanks for clarification @ShaMan123 , will adapt my code to v6 then

@adarsh-drishya
Copy link

adarsh-drishya commented Oct 23, 2023

is there any way i could skip installing jsdom and canvas , it mentions it as required dep but since i am working in browser in react frontend project. i dont want to install canvas and jsdom 5.3.0-browser installs the deps

@asturur
Copy link
Member

asturur commented Oct 26, 2023

5.3.0-browser has this package.json

https://www.npmjs.com/package/fabric/v/5.3.0-browser?activeTab=code

with no dependencies.
How is that you are installing them?

Are you sure those aren't stuck in your yarn.lock or package.lock files from previous attemptes?

@ShaMan123
Copy link
Contributor

I will repeat myself.
Using SSR is possible only in v6
v5 will assume you are in node and import the deps

@asturur
Copy link
Member

asturur commented Oct 26, 2023

yes that i know, but i was oh i was answering the part in which @adarsh-drishya doesn't want to install the deps on 5.3.0, seems unrelated to the original question.
He is using react ( not nextjs ).

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

No branches or pull requests

6 participants