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

fix(dts-plugin): let dts_plugin handle remote paths instead of abs URLs #2478

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

adamdharrington
Copy link

Description

This prevents a type error when a remote is structured with a path reference (/someLocation/mf-manifest.json) instead of an absolute URL by providing a base to the URL constructor. The arbitrary base is removed prior to returning in the case where it is used.

Unhandled Rejection Error:  TypeError: Invalid URL

Related Issue

fix #2406

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 11, 2024

🦋 Changeset detected

Latest commit: 8485347

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 16 packages
Name Type
@module-federation/dts-plugin Patch
@module-federation/enhanced Patch
@module-federation/manifest Patch
@module-federation/rspack Patch
@module-federation/nextjs-mf Patch
@module-federation/node Patch
3008-runtime-remote Patch
@module-federation/modernjs Patch
@module-federation/runtime Patch
@module-federation/webpack-bundler-runtime Patch
@module-federation/sdk Patch
@module-federation/runtime-tools Patch
@module-federation/managers Patch
@module-federation/third-party-dts-extractor Patch
@module-federation/devtools Patch
@module-federation/utilities Patch

Not sure what this means? Click here to learn what changesets are.

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

Copy link

netlify bot commented May 11, 2024

Deploy Preview for module-federation-docs ready!

Name Link
🔨 Latest commit 8485347
🔍 Latest deploy log https://app.netlify.com/sites/module-federation-docs/deploys/663fb52c95f4580008e3ebad
😎 Deploy Preview https://deploy-preview-2478--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 site configuration.

@zackarychapple
Copy link
Collaborator

Love seeing PR's from the community with tests included.

@ScriptedAlchemy
Copy link
Member

@adamdharrington we just introduced publicPath: auto support for mf manifest and dts. Does this solve your problem?

@adamdharrington
Copy link
Author

Sorry for the delay, publicPath: 'auto' doesn't seem to help, firstly because in my case I'm setting a public path for production but also even while testing it does seem to throw at the same location.

usecase:

  • host app deployed to example.com/host (publicPath '/host')
  • remote deployed to example.com/remote (publicPath '/remote')
  • host reference to remote module-federation manifest:
    "remote": "remote@/remote/mf-manifest.json"

In this scenario we could deploy to staging.dev and everything would work as expected, equally we could serve the host locally as on localhost:3333/host and proxy requests to /remote to localhost:4444/remote and all should work.

Currently it seems the only thing that doesn't is that URL constructor in the dts library

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.

TypeError: Invalid URL at buildZipUrl with path instead of absolute url
3 participants