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

feat(fetch-hook): extending fetch hook usage #2046

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

Conversation

SapientMatt
Copy link

@SapientMatt SapientMatt commented Jan 29, 2024

Description

Update to use the fetch hook, if specified, when fetching remoteEntries and chunks on node, in order to achieve similar effect to what was possible with webpackChunkLoad before.

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.

@SapientMatt SapientMatt marked this pull request as ready for review January 29, 2024 14:15
@SapientMatt
Copy link
Author

SapientMatt commented Jan 29, 2024

This is a rather rough bare minimum proposal, that resolved the specifc issues I was having. Namely needing to switch out fetch for Axios, or just to extend fetch, when doing any ssr network calls.
Updating to make the new strategies work with webpackChunkLoad would also work for me, but I thought extending the new hook was likely better.

@ScriptedAlchemy
Copy link
Member

can you not use createScript hook?

@ScriptedAlchemy
Copy link
Member

@zhoushaw any comments here?

@SapientMatt
Copy link
Author

SapientMatt commented Feb 1, 2024

@ScriptedAlchemy I tried using createScriptHook initially, but it doesn't actually seem to be used on the node side of things.
createScriptNode calls createScriptHook, but then only uses the result to reassign the url value, and then uses fetch to fetch that url.
On the node generation side, httpVmStrategy uses the node http library with no apparent way to override either.

@ScriptedAlchemy
Copy link
Member

If create script hook isnt working on node side then we should just need to ammend the source code so it does call the hook, dont think we need to add another api for this if we can make the existing one work node side

Copy link
Contributor

Stale pull request message

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.

None yet

3 participants