-
Notifications
You must be signed in to change notification settings - Fork 236
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
chore: add nx to the repo #65
base: main
Are you sure you want to change the base?
Conversation
Even though you say this doesn't really suite the best use case scenario for the blues stack, I'm already really impressed. I think this is very cool. It's not that we need Nx to help with development of the stack. What we're doing here is making it so when people generate a Remix stack the output is a project pre-configured with Nx so people building their remix app have the best possible experience with the CLI during development. So Nx isn't for us developing the template, it's for everyone else using the template. I think this is a nice step up! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you so much for taking the time to do this. I really like what I'm seeing here.
To be clear, this is a template project which people use to create new Remix apps. So anything in here that needs to be unique per user and per project needs to be accounted for in the remix.init/index.js
script. Additionally, we should probably avoid including anything people need an account for by default.
I think there's a lot of benefit in starting people off with a basic nx config just by virtue of it being a great process manager. They can add more if they want to.
Made the changes:
Nx most of the time is used for managing repos with multiple projects in them. There is no restriction on what that name is. You change the name to any value. You just need to make sure to change the following in nx.json to the same value:
|
This is looking good. I've updated things a bit to make the name that shows up in the output be what they called their app. One issue I bumped into is I did the following:
And I noticed that it didn't rebuild the |
|
||
/.idea |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be in the global .gitignore
https://docs.github.com/en/get-started/getting-started-with-git/ignoring-files#configuring-ignored-files-for-all-repositories-on-your-computer
/.idea |
@@ -95,5 +95,39 @@ | |||
}, | |||
"prisma": { | |||
"seed": "ts-node --require tsconfig-paths/register prisma/seed.ts" | |||
}, | |||
"nx": { | |||
"targets": { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@vsavkin If I'm correct, this can be in the nx.json
/project.json
file as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed. If we're going to have the other config files then let's keep all config in those files.
"@iarna/toml": "^2.2.5", | ||
"sort-package-json": "^1.55.0" | ||
} | ||
"dependencies": {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file can be removed completely I think, since we don't have any dependencies anymore?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, we can probably remove this 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately, we can't remove until remix-run/remix#3146 is merged/released
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Released in 1.5.0
@vsavkin could you bring this PR up to date and go through the remarks? Thank you 🙏🏼 |
5dd109b
to
9709d0a
Compare
@vsavkin Could you please rebase on latest |
Here's a sample remix.config.js I'm using to define routes in each Nx lib project under It's not thoroughly tested yet but maybe others using Nx will find it useful. You could consider adding the watchPaths example to this PR. For the below to work you will need to patch out the code in emptyModulesPlugin that limits it to working on files under const { createGlobPatternsForDependencies } = require("@nrwl/react/tailwind");
const { defineConventionalRoutes } = require("@remix-run/dev/dist/config/routesConvention");
const path = require("path");
const fs = require("fs");
/**
* @type {import('@remix-run/dev').AppConfig}
*/
module.exports = {
// ignore all files in routes folder
ignoredRouteFiles: ["**/.*"],
// add routes for all referenced lib projects
routes: defineLibraryRoutes,
// watch referenced libs for file changes during dev
watchPaths: () => createGlobPatternsForDependencies(__dirname, "/**"),
};
async function defineLibraryRoutes() {
let routeManifest;
const appFolder = path.join(__dirname, "app");
const libDirectories = createGlobPatternsForDependencies(__dirname, "/lib/");
for (const dir of libDirectories) {
// check if referenced library has routes defined
const routesFolder = path.join(dir, "routes");
if (!fs.existsSync(routesFolder)) continue;
// get all routes from library based on default conventions
let manifest = defineConventionalRoutes(dir, ["**/*.test.{js,jsx,ts,tsx}"]);
// convert route file paths to be relative to app directory
for (const [, route] of Object.entries(manifest)) {
const routeFile = path.join(dir, route.file);
route.file = path.relative(appFolder, routeFile);
}
routeManifest = { ...routeManifest, ...manifest };
}
return routeManifest;
} |
@vsavkin Are you still interested in adding NX to the repo? |
This PR is a follow-up of my conversation with Kent.
This PR shows how to add Nx to this repo, make some operations faster, and improve some dev ergonomics.
Watch this video to see how everything works now: LINK
I intentionally didn't change anything about the repo itself to make it less overwhelming. So I want to mention that this isn't the best way to use Nx (or any such tool really). For instance, cypress and prisma can be made separate from app, which would make the dev ergonomics nicer etc.
This repo is also small, so both DX and performance advantages Nx offers aren't really that important here. The build command is fast enough without caching and distribution, and the output of say "validate" is understandable without the niceties Nx and Nx Cloud offer. It would have been easier to show the before and after if, say, all the stacks were in the same repo.