Skip to content

feat(@react-router/dev): add defineConfig utility #12541

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

Open
wants to merge 2 commits into
base: dev
Choose a base branch
from

Conversation

AlemTuzlak
Copy link
Contributor

Create a vite style defineConfig so you can do something like:

import { defineConfig } from "@react-router/dev/config";

export default defineConfig({
  // stuff
})

Copy link

changeset-bot bot commented Dec 13, 2024

⚠️ No Changeset found

Latest commit: d85aed1

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@AlemTuzlak AlemTuzlak changed the title Feat/add define config feat: add define config Dec 13, 2024
@MichaelDeBoey MichaelDeBoey changed the title feat: add define config feat(@react-router/dev): add defineConfig utility Dec 13, 2024
Copy link
Member

@markdalgleish markdalgleish left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We went back and forth on whether to add this, but we ultimately decided against it because it adds needless indirection, even though the syntax does look nicer to my eyes.

A defineConfig function arguably makes it look like there's some kind of runtime behaviour supported here, or that it may be introduced in the future, but if it's only used to perform type checking, it felt simpler and more transparent to simply annotate the type.

Curious to hear your thoughts on this though.

@AlemTuzlak
Copy link
Contributor Author

@rossipedia pinging you so you can leave thoughts as well, but here are mine:

I think this is more of a "standards" thing in my eyes, as the react-router plugin relies on Vite, and wraps it with with a custom command I'd expect for it to have similar config definition to Vite, something like Vitest does, I understand they integrate tightly and are built by the same people, but I feel like this function definition has become something you expect to see in the Vite ecosystem.

Another thing I'd like to mention is that there are certain config flags, even though they don't provide runtime behaviors, they do provide buildtime behaviors like onBuildEnd etc. I agree that at the moment there is no runtime behavior but you never know if there will be some, also this would easily allow you to wrap the Vite defineConfig and do something like:

import { defineConfig } from "@react-router/dev/config";

export default defineConfig({
  vite: {
     // ... vite stuff here ...
  },
  future: {
    // RSC WHEN?!?!?
  }
})

I personally feel like it's just a convinience 1 liner utility that feels warm and familiar compared to 1 million custom configs like eslint, biome, insert random json config file here. It takes 0 maintainence as it just inherits the type you have to define and manage anyway and it feels more in line with Vite.

@rossipedia
Copy link
Contributor

So yeah, I agree with @AlemTuzlak on the "standards" thing, although I'd probably phrase it more like a "comfortable convention".

Even though it's a one liner, the goal is to improve the DX of configuring things, rather than imply that it's doing more than just type-coercion. Since RR framework relies on vite, and vite itself exposes a NOOP defineConfig helper, I think it's reasonable to expect that an engineer would understand that defineConfig isn't doing anything special and behaves the same way (Principle of Least Surprise, really).

The benefits are tangible, IMO though, rather than philosophical. Imagine a dev finds themselves where they need to create a react-router.config.ts file from scratch. I think it's reasonable to assume the first thing they'd do is look for something like defineConfig, because it's a mnemonic they've seen and/or used in other vite-based projects.

Autocomplete in VSCode shows more than just Config exported from @react-router/dev/config, and even though Config is the most likely type that a user wants, a dev who's used vite before but might be trying out react-router will feel much more at home with a defineConfig helper, I think, rather than going "Ok I have a Config type, what do I do with it? Does react-router need a default export? Do I need to pass that to something else? I just wanna get started"

image

Using defineConfig allows the to re-use the concepts/abstractions they've already become familiar with in the vite ecosystem, which makes them feel immediately "at home" and comfortable, because RR is predictable to configure.

They see defineConfig and think "Oh, there's a defineConfig! Sweet, lemme just call that and export it as default because that's what everybody else does" and things just work as they expect, leaving you with a much happier and confident dev, and a net positive experience with React Router.

Some samples of packages that expose this pattern (definitely not exhaustive):

// vite
import { defineConfig } from 'vite';
export default defineConfig({
  ...
});

// vitest
import { defineConfig } from 'vitest/config';
export default defineConfig({
  ...
});

// playwright
import { defineConfig } from '@playwright/test';
export default defineConfig({
  ...
});

// solid start
import { defineConfig } from "@solidjs/start/config";
export default defineConfig({
  ...
});

// cypress
import { defineConfig } from 'cypress'
export default defineConfig({
  ...
})

// sanity
import {defineConfig} from 'sanity'
export default defineConfig({
  ...
})

// etc...

IMO, from what I can see this is a fairly established pattern to provide easier type-assistance for configuration. Personally, I find defineConfig() nicer to read and use than having something like:

import type { Config } from 'lib';

// feels brittle for some reason
export default {

} as Config; // or possibly `satisfies Config`

// also kinda yucky, having two syntactical constructs
let config: Config = {
  ...
};

export default config;

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.

4 participants