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

@nx/rspack:rspack executor does not use baseHref option #28455

Closed
1 of 4 tasks
scott-gmr opened this issue Oct 15, 2024 · 3 comments · Fixed by #28660
Closed
1 of 4 tasks

@nx/rspack:rspack executor does not use baseHref option #28455

scott-gmr opened this issue Oct 15, 2024 · 3 comments · Fixed by #28660
Assignees
Labels
outdated scope: bundlers Issues related to webpack, rollup type: bug

Comments

@scott-gmr
Copy link

scott-gmr commented Oct 15, 2024

Current Behavior

The @nx/rspack:rspack executor ignores the baseHref option. The output index.html file does not have the <base href="<baseHref>" /> tag when this option is provided.

I ended up using this code to make a bandaid for this in rspack.config.js:

module.exports = composePlugins(withNx(), withReact(), (config) => { 
  config.plugins.find(
    (plugin) => plugin.constructor.name === 'HtmlRspackPlugin'
  )._args[0].base = { href: '/my-base-href/' };

  return config;
});

Expected Behavior

When provided, baseHref should set the <base href="<baseHref>"> tag in the output HTML document.

GitHub Repo

https://github.com/scott-gmr/nx-base-href-bug

Steps to Reproduce

  1. Use the @nx/rspack:rspack executor with the baseHref option set.
  2. Check the output index.html file.

Nx Report

Node : 20.14.0
OS : darwin-arm64
Native Target : aarch64-macos
npm : 10.7.0

nx : 20.0.0
@nx/js : 20.0.0
@nx/jest : 20.0.0
@nx/eslint : 20.0.0
@nx/workspace : 20.0.0
@nx/devkit : 20.0.0
@nx/eslint-plugin : 20.0.0
@nx/react : 20.0.0
@nx/rspack : 20.0.0
@nx/web : 20.0.0
typescript : 5.5.4

Registered Plugins:
@nx/eslint/plugin
@nx/jest/plugin

Failure Logs

No response

Package Manager Version

10.7.0

Operating System

  • macOS
  • Linux
  • Windows
  • Other (Please specify)

Additional Information

This is documented as a valid option, but it isn't used.

It is fairly plain to see why this occurs just by looking at Nx's code directly. This code search shows that the option is only even referenced in the @nx:rspack/dev-server executor's package.

It can also been seen here that the HtmlRspackPlugin is only given the template option with no base set, and this is the only time HtmlRspackPlugin occurs in the source code.

I would also say that often options for CLI commands and generators/executors are often under-documented. "Base url for the application being built" is a pretty vague and redundant description for this option. I would assume this has the same intention as the baseHref option previously working as expected for Nx's Webpack package, but the description says nothing about what this option is supposed to actually affect. When this appears alongside the similarly vaguely described deployUrl, it makes it confusing for a developer to know what the intention of these options even are, especially when the options themselves are not taking effect.

I try not to jump to blaming my libraries' code for issues I'm experiencing, but to be honest I have had to dig in Nx's node_modules frequently, more frequently than any other libraries, and this is not the first time my issue has turned out to be an Nx bug (or a sneaky breaking change with no release notes in a minor/patch release), but it's the first time no one seems to have created the same issue. Is there no testing to make sure that at minimum the options passed to these generators are being even basically utilized? I would think if you were to test anything it would at least be the public interface of these tools. It seems the push is to migrate to Rspack from Webpack, yet feature parity feels very sloppy.

@Coly010 Coly010 self-assigned this Oct 16, 2024
@Coly010 Coly010 added the scope: bundlers Issues related to webpack, rollup label Oct 16, 2024
@Coly010
Copy link
Contributor

Coly010 commented Oct 16, 2024

@scott-gmr I'm sorry you're frustrated.

The @nx/rspack plugin was a labs project until this release. Because of that, it never received the same attention our other first-party plugins received.

Now that we have promoted it to first-party status, it will receive better care and attention. Part of that will be to bring it in line with our @nx/webpack offering.

You seem to have a good understanding of the issue at hand, so feel free to open a PR to fix it. Otherwise, I will address it as soon as I can

@iconag-bbasmer
Copy link

Please don't get me wrong, I really like NX and all you're doing here. But still: The change to rspack has taken at least 2 weeks of my time for research and fixes to make things run again as before and still I couldn't move my changes to production because I couldn't figure out the issue described above. In my opinion it should be obvious that as soon as you move a plugin to "first-party" status it should be made sure that all the functionality works as expected before even releasing it instead of letting your users test and fix things that should come from you.

Just my .2 cents...

Do you have a timeline when this issue will be fixed?

Thanks!

jaysoo pushed a commit that referenced this issue Oct 31, 2024
<!-- Please make sure you have read the submission guidelines before
posting an PR -->
<!--
https://github.com/nrwl/nx/blob/master/CONTRIBUTING.md#-submitting-a-pr
-->

<!-- Please make sure that your commit message follows our format -->
<!-- Example: `fix(nx): must begin with lowercase` -->

<!-- If this is a particularly complex change or feature addition, you
can request a dedicated Nx release for this pull request branch. Mention
someone from the Nx team or the `@nrwl/nx-pipelines-reviewers` and they
will confirm if the PR warrants its own release for testing purposes,
and generate it for you if appropriate. -->

## Current Behavior
<!-- This is the behavior we have today -->
The `@nx/rspack:rspack` executor supports `baseHref` as an option, but
we do not set it in the `rspack.HtmlRspackPlugin`.


## Expected Behavior
<!-- This is the behavior we should expect with the changes in this PR
-->
Ensure if `baseHref` is set, we pass it to `rspack.HtmlRspackPlugin`

## Related Issue(s)
<!-- Please link the issue being fixed so it gets closed when this is
merged. -->

Fixes #28455
Copy link

This issue has been closed for more than 30 days. If this issue is still occuring, please open a new issue with more recent context.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 28, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
outdated scope: bundlers Issues related to webpack, rollup type: bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants