Skip to content

Unnecessary polyfill for http and https required #507

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

Closed
PSanetra opened this issue Aug 27, 2022 · 3 comments
Closed

Unnecessary polyfill for http and https required #507

PSanetra opened this issue Aug 27, 2022 · 3 comments
Assignees
Labels
module/browser Module capabilities in a browser environment status/invalid This doesn't seem right status/no-issue-activity

Comments

@PSanetra
Copy link
Contributor

PSanetra commented Aug 27, 2022

Describe the Bug
Compiling a react app with this library as dependency results in a compilation error.

Steps to Reproduce

  1. Create a react app using the create-react-app tooling
  2. Add cloudevents as dependency
  3. Compile project

Compiling the project results in the following error message:

ERROR in ./node_modules/cloudevents/dist/transport/http/index.js 18:31-46
Module not found: Error: Can't resolve 'http' in '/home/[...]/node_modules/cloudevents/dist/transport/http'

BREAKING CHANGE: webpack < 5 used to include polyfills for node.js core modules by default.
This is no longer the case. Verify if you need this module and configure a polyfill for it.

If you want to include a polyfill, you need to:
        - add a fallback 'resolve.fallback: { "http": require.resolve("stream-http") }'
        - install 'stream-http'
If you don't want to include a polyfill, you can use an empty module like this:
        resolve.fallback: { "http": false }

ERROR in ./node_modules/cloudevents/dist/transport/http/index.js 20:32-48
Module not found: Error: Can't resolve 'https' in '/home/[...]/node_modules/cloudevents/dist/transport/http'

BREAKING CHANGE: webpack < 5 used to include polyfills for node.js core modules by default.
This is no longer the case. Verify if you need this module and configure a polyfill for it.

If you want to include a polyfill, you need to:
        - add a fallback 'resolve.fallback: { "https": require.resolve("https-browserify") }'
        - install 'https-browserify'
If you don't want to include a polyfill, you can use an empty module like this:
        resolve.fallback: { "https": false }

Expected Behavior
It should be possible to compile the project including this library as dependency in a browser project without the need for additional webpack configuration.

Additional Context
I know that there is some kind of workaround by configuring webpack, but the create-react-app tool does not support modifying the webpack configuration and the maintainers discourage from doing this: facebook/create-react-app#11756 (comment)

@lance
Copy link
Member

lance commented Sep 2, 2022

@PSanetra thanks for reporting this. We do generate browser friendly code with these polyfills set to resolve.fallback { "https": false } as recommended. You can use this in your react app, by using the bundle like so.

import { CloudEvent, HTTP, ...} from "cloudevents/bundles/cloudevents";

I tried this locally and it seemed to work fine. However, I am not really much of a front end dev, so I'm not sure if this is really the best way to do it. Please let me know if this works for you. And if you have suggestions for a better way to distribute these web bundles, I'm all ears.

@lance lance self-assigned this Sep 2, 2022
@lance lance added the module/browser Module capabilities in a browser environment label Sep 2, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Oct 3, 2022

This issue is stale because it has been open 30 days with no activity.

@lance
Copy link
Member

lance commented Oct 3, 2022

Closing as not reproducible.

❯ npm install --save cloudevents

added 10 packages, and audited 1461 packages in 2s

215 packages are looking for funding
  run `npm fund` for details

6 high severity vulnerabilities

To address all issues (including breaking changes), run:
  npm audit fix --force

Run `npm audit` for details.

sdk-javascript/testapp on  main [$!?] via  v18.10.0 on ☁️  [email protected] took 2s
❯ npm run build

> [email protected] build
> react-scripts build

Creating an optimized production build...
Compiled successfully.

File sizes after gzip:

  46.6 kB  build/static/js/main.8815826c.js
  1.78 kB  build/static/js/787.2746eaee.chunk.js
  541 B    build/static/css/main.073c9b0a.css

The project was built assuming it is hosted at /.
You can control this with the homepage field in your package.json.

The build folder is ready to be deployed.
You may serve it with a static server:

  npm install -g serve
  serve -s build

Find out more about deployment here:

  https://cra.link/deployment


@lance lance closed this as completed Oct 3, 2022
@lance lance added the status/invalid This doesn't seem right label Oct 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
module/browser Module capabilities in a browser environment status/invalid This doesn't seem right status/no-issue-activity
Projects
None yet
Development

No branches or pull requests

2 participants