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

process is not defined using comunica and react without editing the webpack #1097

Closed
BenoitLan opened this issue Oct 10, 2022 · 7 comments · Fixed by RubenVerborgh/AsyncIterator#94

Comments

@BenoitLan
Copy link

Hello I'm trying to use comunica with a standard react app (created with for example npx create-reactapp my-app) without editing the webpack (when for example npm run eject is used to edit the webpack).

Comunica works perfect with react when you edit the webpack and use "node-polyfil-webpack-plugin". But gives the following error when a standard react app is used:
image

I'm looking at this problem for a while but can't really figure out how to fix it. I'm currently comparing the 2 app's with each other.
Both use the newest version of comunica. So again, I want to change the code of comunica in order to work, but not that of the react app itself.

I already scoured the internet for this problem, but none really worked.
I've tried to add the following code in App.js:
window.process = {
env: {
NODE_ENV: 'development'
}
}
which solves the issue but gives a new error: process.tick is not defined, so it must be solved in another way.
Any help or tips would be welcome!

@github-actions
Copy link

Someone will answer your question soon. In the meantime, you might be able to get help more quickly on our Gitter channel.

@rubensworks
Copy link
Member

I knew there were issues with buffer (which will be solved soon rubensworks/sparqljson-parse.js#42), but I didn't know about any issues with process.

Are you sure that your installed version of readable-stream is at least 4.2.0?

Also including @surilindur in this thread.

@BenoitLan
Copy link
Author

Yes readable-stream in version 4.2.0, so that is not the problem I'm afraid.

@rubensworks
Copy link
Member

Just looked into this a bit, and it looks like this is a create-react-app bug in their webpack config.

This problem is caused by require("asynciterator") returning a string (such as /static/media/asynciterator.e7f1dad96cda58a54964.cjs).
This is caused by a misconfiguration in React's default Webpack config: facebook/create-react-app#11889
Modifying AsyncIterator's package.json to refer to the .js index file instead of .cjs resolves the issue.

Even though there are some PRs open to create-react-app since early this year that fix the problem, they have not been merged yet (facebook/create-react-app#12605 and facebook/create-react-app#12021).
So I'm not sure when (if ever) this will be resolved on their end.

So this boils down to tooling not having decent ESM support yet...

@RubenVerborgh Would you be open to making changes to the way AsyncIterator is published? This would allow us to fix this problem without having to wait for those create-react-app PRs to be merged.

@RubenVerborgh
Copy link
Contributor

Fine with me! And we should definitely also take this to the next major version.

@rubensworks
Copy link
Member

@RubenVerborgh I was thinking of just publishing ESM files as .mjs, and CJS as .js (instead of .js and .cjs respectively). I suspect this won't require a major version bump.

@RubenVerborgh
Copy link
Contributor

Nah it should be fine; and in any case our default for the next major (which is currently on a fork).

rubensworks added a commit to rubensworks/AsyncIterator that referenced this issue Nov 8, 2022
This fixes issues with the commonly-used create-react-app tool that is
unable to handle .cjs files.

This change consider CJS by default for .js files, but still exposes ESM
for the tools that have proper support for it.

See facebook/create-react-app#11889

Closes comunica/comunica#1097
RubenVerborgh pushed a commit to RubenVerborgh/AsyncIterator that referenced this issue Nov 8, 2022
@rubensworks rubensworks moved this to Done in Maintenance May 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

3 participants