nrwl / nx

Smart Monorepos · Fast CI
https://nx.dev
MIT License
23.35k stars 2.33k forks source link

Release 19.6.6 can break remix filename conventions for ".server" or ".client". #27966

Open skindc opened 1 week ago

skindc commented 1 week ago

Current Behavior

This issue is more for awareness of a possible breaking change rather than a bug report.

When upgrading from 19.6.5 to 19.6.6 the builds for my Remix Vite projects was broken. As per Remix conventions I have server only files isolated using "xx.server.ts" naming. When building with 19.6.6 the build fails with Vite Rollup errors saying the module can not be found. In my case my module file would be services/webhook.server.js and the import using none ESM import import { foo } from "services/webhook.server" without extension. I suspect this is caused by PR #27774. I resolved this matter by changing my import statement to be more along the lines of ESM expectations and added the .js extension, ie import { foo } from "services/webhook.server.js"

It should be noted I am not using the @nx/vite plugin for the build target of my Vite Remix project, and I am using a script target in a package.json for the project. This is because I was previously finding that the @nx/vite did not play nice with the more recent developments of Remix in using Remix as a plugin for vite. I need to investigate if this is more cohesive in recent releases.

Expected Behavior

I would expect that the use of ".server" notation in import statements be respected and not necesserily considered as the extension of which I feel is the suspect case here from the aforementioned PR.

GitHub Repo

No response

Steps to Reproduce

  1. Create NX Mono repo with one Vite project and Vite using the remix plugin.
  2. Have one route that has an import from a module using the .server Remix file naming convention.
  3. The import of the .server should be using the project path notation using the tsconfig-base.json paths. ie import serverModule from "@myapp/services/module.server" without the js extension.

The build will fail with soemthing similar to:

[vite]: Rollup failed to resolve import "@myapp/services/webhook.server"

Change the import statement to include the js extension, and then the build will succeed.

Nx Report

NX   Report complete - copy this into the issue template

Node           : 22.8.0
OS             : darwin-arm64
Native Target  : aarch64-macos
npm            : 10.8.2

nx (global)        : 19.6.6
nx                 : 19.6.6
@nx/js             : 19.6.6
@nx/jest           : 19.6.6
@nx/linter         : 19.6.6
@nx/eslint         : 19.6.6
@nx/workspace      : 19.6.6
@nx/devkit         : 19.7.3
@nx/esbuild        : 19.6.6
@nx/eslint-plugin  : 19.6.6
@nx/node           : 19.6.6
@nx/react          : 19.6.6
@nrwl/tao          : 19.6.6
@nx/vite           : 19.6.6
@nx/web            : 19.6.6
typescript         : 5.5.4
---------------------------------------
Registered Plugins:
@nx/eslint/plugin
@nx/jest/plugin
---------------------------------------
Community plugins:
@nx-tools/nx-container : 6.0.1
---------------------------------------
The following packages should match the installed version of nx
  - @nx/devkit@19.7.3
  - @nrwl/devkit@19.7.3

Failure Logs

[vite]: Rollup failed to resolve import "@myapp/services/webhook.server"

Package Manager Version

No response

Operating System

Additional Information

No response

Coly010 commented 1 week ago

Hey @skindc

Thanks for raising the issue.

Two things to touch on to begin with.

  1. Your @nx/* packages are not all aligned. This could result in some erroneous behaviour. It might not be contributing directly to the problem you're facing but you could encounter further issues. The main offender appears to be @nx/devkit which is using version 19.7.3 while the other packages are on 19.6.6.

  2. You wouldn't use @nx/vite directly to build your app in the case of remix. However, our @nx/remix/plugin will now infer targets correctly for Remix Vite set ups. Therefore, you can add the plugin to your nx.json#plugins array and it should be able to infer the targets correctly for your app. You just need to add the following to your nx.json

{
 ...,
 "plugins": [
   ...,
   "@nx/remix/plugin"
 ]
}

You might need to also remove the build script from your package.json, otherwise it would overwrite what is inferred by Nx.

I know you've provided some repro steps, but to remove any ambiguity in how I might generate a vite project and set up the tsconfig paths, could you provide a GH repo with a minimal reproduction? It would help me investigate this a lot easier.

Thank you

skindc commented 1 week ago

Hi @Coly010

Thank you for your response. 1) Thank you for this observation, I did notice this but forget to mention this in my original post. I do not install @nx/devkit directly. It seems this is a dependency of @nx-tools/container-metadata@6.0.2. And I will look into how to keep this aligned as I appreciate the concern you mentioned. 2) Yes I have observed more recent releases and I intend to attempt using @nx/remix/plugin to resolve previous issues I was facing. I am very keen to use NX integrated plugins where possible as I was also concerned of the usage of scripts in package json and its affect on inference of NX plugins and NX in general..

I will attempt to get the minimal GH repo set up for reproduction of the issue in the day or so. Sorry for inconvenience of not providing that immediately.

Thanks

github-actions[bot] commented 4 days ago

This issue has been automatically marked as stale because no reproduction was provided within 7 days. Please help us help you. Providing a repository exhibiting the issue helps us diagnose and fix the issue. Any time that we spend reproducing this issue is time taken away from addressing this issue and other issues. This issue will be closed in 21 days if a reproduction is not provided. If a reproduction has been provided, please reply to keep it active. Thanks for being a part of the Nx community! 🙏