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

vite_react_refresh_tag does not respect base setting #515

Open
drkstr101 opened this issue Nov 22, 2024 · 1 comment
Open

vite_react_refresh_tag does not respect base setting #515

drkstr101 opened this issue Nov 22, 2024 · 1 comment
Labels
bug Something isn't working

Comments

@drkstr101
Copy link

Description 📖

The script module appended by the vite_react_refresh_tag does not consider the base setting in the react preamble code.

Reproduction 🐞

  1. Start with the rails example app
  2. Add the following to config/environments/development.rb
    config.relative_url_root = '/sub'
    config.action_controller.relative_url_root = '/sub'
  3. Map the subpath in your config.ru
    require_relative 'config/environment'
    
    map '/sub' do
      run Rails.application
      Rails.application.load_server
    end
  4. Add base setting to config/vite.json
    {
      "all": {
        "base": "/sub",
        ...
      }
    }
  5. Now run the app in development mode with the /sub path loaded in the browser.

The appended module script will look something like so:

<script type="module">
//<![CDATA[
import RefreshRuntime from '/vite-dev/@react-refresh'
RefreshRuntime.injectIntoGlobalHook(window)
// ...
//]]>
</script>

This leads to a 404 error on the import. The correct import path should be.

<script type="module">
//<![CDATA[
import RefreshRuntime from '/sub/vite-dev/@react-refresh'
RefreshRuntime.injectIntoGlobalHook(window)
// ...
//]]>
</script>

I verified that navigating the correct path returns the module source as expected.

The prefix_asset_with_host method at vite_ruby/manifest.rb#L144 appears to be the issue at first glance. I might be able to use the assetHost setting, but setting it to /sub broke the other imports. Configuring the entire host URL would work, but it is undesirable.

Could config.base be included in this join as well? If desired, I could submit a PR with a test case.

@drkstr101 drkstr101 added the bug: pending triage Something doesn't seem to be working, but hasn't been verified label Nov 22, 2024
@ElMassimo ElMassimo added bug Something isn't working and removed bug: pending triage Something doesn't seem to be working, but hasn't been verified labels Nov 22, 2024
@ElMassimo
Copy link
Owner

ElMassimo commented Nov 22, 2024

Hi Aaron, thanks for reporting.

Can you confirm if the following patch fixes the issue in your scenario:

ViteRuby::Manifest.prepend Module.new {
  # Internal: Prefixes an asset with the `asset_host` for tags that do not use
  # the framework tag helpers.
  def prefix_asset_with_host(path)
    File.join(vite_asset_origin || config.asset_host || "/", config.base, config.public_output_dir, path)
  end
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants