Skip to content

🐛 getBundles shouldn't require webpack #31

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

Conversation

trhinehart-godaddy
Copy link
Contributor

@trhinehart-godaddy trhinehart-godaddy commented Oct 1, 2021

Summary

The below import is failing in production

import { getBundles } from 'react-loadable-ssr-addon';
17:57:36   Error: Cannot find module 'webpack/package.json'
17:57:36   Require stack:
17:57:36   - /src/node_modules/react-loadable-ssr-addon/lib/ReactLoadableSSRAddon.js
17:57:36   - /src/node_modules/react-loadable-ssr-addon/lib/index.js

Why

This library is exported as CommonJS, which means treeshaking is not supported. When importing getBundles from the index.js as described in the README, the module ReactLoadableSSRAddon is also loaded which was requiring webpack (a devDependency) during server execution in production (where devDependencies are pruned).

A better fix would be to export this library to ESM as well, and define a "module": "./esm/index.js" export in the package.json, along with a "sideEffects": false to allow proper importing of named exports.

Workaround

To workaround this issue, I had to change my import to

import getBundles from 'react-loadable-ssr-addon/lib/getBundles';

Checklist

  • Your code builds clean without any errors or warnings
  • You are using approved terminology
  • You have added unit tests, if apply.

Emojis for categorizing pull requests:

⚡️ New feature (:zap:)
🐛 Bug fix (:bug:)
🔥 P0 fix (:fire:)
✅ Tests (:white_check_mark:)
🚀 Performance improvements (:rocket:)
🖍 CSS / Styling (:crayon:)
♿ Accessibility (:wheelchair:)
🌐 Internationalization (:globe_with_meridians:)
📖 Documentation (:book:)
🏗 Infrastructure / Tooling / Builds / CI (:building_construction:)
⏪ Reverting a previous change (:rewind:)
♻️ Refactoring (like moving around code w/o any changes) (:recycle:)
🚮 Deleting code (:put_litter_in_its_place:)

@trhinehart-godaddy trhinehart-godaddy changed the title 🐛 getBundles should require webpack 🐛 getBundles shouldn't require webpack Oct 1, 2021
@themgoncalves
Copy link
Owner

Hi, thanks for the contribution and rising up the issue! Sadly your PR would need some adjustments and for that reason, having in mind time constraint, I already made the fix and published it on the v1.0.2.

@trhinehart-godaddy trhinehart-godaddy deleted the patch-3 branch October 4, 2021 15:33
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.

2 participants