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

Update readme.md for use with Webpack 5 #24

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

dbuchet
Copy link

@dbuchet dbuchet commented Jan 10, 2023

No description provided.

@lovasoa
Copy link
Member

lovasoa commented Jan 10, 2023

Maybe you could update the code itself to use the latest react and webpack ?

https://github.com/sql-js/react-sqljs-demo/blob/master/package.json

@dbuchet
Copy link
Author

dbuchet commented Jan 10, 2023

I've started with that, but I think many people are still with Webpack 4. So i've just updated the readme. Tell me what you prefer :)

@lovasoa
Copy link
Member

lovasoa commented Jan 10, 2023

I think we can update the code to the latest versions, and just leave a link in the readme to the version using webpack 4.

I tagged the current state of the repo as v1, so you can link to https://github.com/sql-js/react-sqljs-demo/tree/v1

Comment on lines +13 to +15
'path': require.resolve('path-browserify'),
'crypto': require.resolve('crypto-browserify'),
'stream': require.resolve('stream-browserify')
Copy link
Member

Choose a reason for hiding this comment

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

Do we actually use path, crypto and stream anywhere ?

Copy link
Author

Choose a reason for hiding this comment

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

that a good question, and honestly I don"t really know how webpack handle wasm, nor how sql.js is built.
My point is everything was working fine for me with Webmact4, and when updating to Webpack5 I had 3 successive errors saying "path", "crypto" and "stream" are not included as polyfill (comming from sql.js/dist), , and adding these lines fixed the error and everything run again smoothly

Copy link
Member

Choose a reason for hiding this comment

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

We should probably avoid encouraging users to bundle three dependencies they don't need with their code.

Copy link

Choose a reason for hiding this comment

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

In addition to path, crypto and stream, I've just encountered another error related to fs. It seems that setting the fallback of fs to false got it working for me. Is there a chance other dependencies can be set to false as well?

config.resolve.fallback = {
    'fs': false,
    'path': false,
    'crypto': false,
    'stream': false
  }

Edit: This seems to be working for my other repository using TypeORM + Sql.js + Expo Web! Requires further testing on React + Sql.js though I believe it should work the same.

README.md Show resolved Hide resolved
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

3 participants