Skip to content

fix(nextjs-mf): Try CSP-safe method for getting globalThis first #3776

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

mitchellrj
Copy link

Description

Where unsafe-eval is not permitted in a Content Security Policy, trying the Function approach first then falling back causes unnecessary CSP Reports and errors to be logged by the browser. This approach uses globalThis by default, then attempts to catch the Webpack rewriting of it.

Related Issue

#3772

Types of changes

  • Docs change / refactoring / dependency upgrade
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)

Checklist

  • I have added tests to cover my changes.
  • All new and existing tests passed.
  • I have updated the documentation.

Copy link

changeset-bot bot commented May 15, 2025

⚠️ No Changeset found

Latest commit: 7fde21f

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Copy link

netlify bot commented May 15, 2025

Deploy Preview for module-federation-docs ready!

Name Link
🔨 Latest commit 7fde21f
🔍 Latest deploy log https://app.netlify.com/projects/module-federation-docs/deploys/6846abd496ea740008a342eb
😎 Deploy Preview https://deploy-preview-3776--module-federation-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@mitchellrj mitchellrj changed the title Try CSP-safe method for getting globalThis first fix(nextjs-mf): Try CSP-safe method for getting globalThis first May 15, 2025
@ScriptedAlchemy
Copy link
Member

@mitchellrj run pnpm changeset add

@ScriptedAlchemy
Copy link
Member

@mitchellrj bump

mitchellrj added a commit to mitchellrj/module-federation-core that referenced this pull request May 16, 2025
@mitchellrj
Copy link
Author

@ScriptedAlchemy want me to bring up to date with main again?

@mitchellrj mitchellrj force-pushed the fix-reduce-csp-false-positives branch from bbe342e to 05ff51b Compare May 27, 2025 11:35
mitchellrj added a commit to mitchellrj/module-federation-core that referenced this pull request May 27, 2025
@mitchellrj
Copy link
Author

There's still an issue with the type of __webpack_require__.g not being defined. I'm not so familiar with how to fix that.

@mitchellrj mitchellrj force-pushed the fix-reduce-csp-false-positives branch from a8f9efa to a61b4bd Compare June 5, 2025 15:14
@mitchellrj mitchellrj force-pushed the fix-reduce-csp-false-positives branch from a61b4bd to ee0a638 Compare June 5, 2025 15:15
@ScriptedAlchemy
Copy link
Member

Okay does latest PR seem to work? address previous comments?

@mitchellrj
Copy link
Author

mitchellrj commented Jun 9, 2025

The tests are passing.

I tried building locally to test e2e, but that is where I run into the __webpack_require__.g issue previously described.
image

I think that it is fixed in my changes to webpack/module.d.ts but the CONTRIBUTORS file doesn't give me instructions to get a build working locally to test it myself, so I'm not sure if I'm doing it right. Sorry, I'm quite new to this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants