fix(remix-dev): stop automatically updating tsconfig.json

[SeanGroff]

Closes: #5509

• Docs
• Tests

Testing Strategy:

I opened up my mac machine and ran npm run dev
I didn't have the tsconfig compiler option strict specified. The Remix dev package updates my tsconfig file by adding the compiler option strict: true

Here's a discussion I had on Twitter about this issue: https://twitter.com/_SeanGroff/status/1626637132176424963?s=20

Updating the TSConfig file shouldn't be the default behavior. The user should have to pass a --fix flag to opt into $ remix dev automatically updating the TSConfig file.

[remix-cla-bot]

Hi @SeanGroff,

Welcome, and thank you for contributing to Remix!

Before we consider your pull request, we ask that you sign our Contributor License Agreement (CLA). We require this only once.

You may review the CLA and sign it by adding your name to contributors.yml.

Once the CLA is signed, the CLA Signed label will be added to the pull request.

If you have already signed the CLA and received this response in error, or if you have any questions, please contact us at [email protected].

Thanks!

- The Remix team

[changeset-bot]

🦋 Changeset detected

Latest commit: 10ba614

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 18 packages

Name	Type
@remix-run/dev	Patch
create-remix	Patch
@remix-run/css-bundle	Patch
remix	Patch
@remix-run/architect	Patch
@remix-run/cloudflare	Patch
@remix-run/cloudflare-pages	Patch
@remix-run/cloudflare-workers	Patch
@remix-run/deno	Patch
@remix-run/eslint-config	Patch
@remix-run/express	Patch
@remix-run/netlify	Patch
@remix-run/node	Patch
@remix-run/react	Patch
@remix-run/serve	Patch
@remix-run/server-runtime	Patch
@remix-run/testing	Patch
@remix-run/vercel	Patch

Not sure what this means? Click here to learn what changesets are.

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

[remix-cla-bot]

Thank you for signing the Contributor License Agreement. Let's get this merged! 🥳

[garand]

Interesting post by Wes Bos regarding this behavior... he liked it. https://twitter.com/wesbos/status/1627707679840915456

Maybe it would make sense to notify the user and give them the option to update their config?

[SeanGroff]

While my current PR is an improvement, I think we can iterate on this solution to strike a happy middle ground between ease of use and being too forceful:

💡We keep the "Mandatory/Required" TSConfig compiler suggestions and "Recommended" compiler suggestions. However, we no longer update the TSConfig. We display our suggestions in the CLI. This provides the developer the freedom to configure the TSConfig themselves. If issues arise, maybe consider the suggestions from Remix 😀. Then we add a flag to the Remix Config that will disable the TSConfig suggestions in the terminal if the developer prefers.

This should also be documented, depending on the direction we decide to go.

It's important to keep in mind the scope of this PR as well. This refers to the Remix Dev server, NOT the Create Remix App generator.

[SeanGroff]

We could also prompt the developer with these changes in the CLI and they can provide input Y/n to apply the changes to the TSConfig.

My worry here is in a team environment. A developer may not know the full implications of this change. They submit a PR, the CI fails or a Senior engineer asks them to revert their TSConfig changes.

On dev teams, updating the tooling should be intentional and planned out. Preferably decoupled from any feature or bugfix work.

[chaance]

I'd suggest that we keep the warning but allowing users to opt-in to automatic changes with --fix or something. That said, the warning should also be dismiss-able. Probably beyond the scope for this PR, but if we can leave the log in place as a warning for now I think that's a step in the right direction.

[SeanGroff]

I'd suggest that we keep the warning but allowing users to opt-in to automatic changes with --fix or something. That said, the warning should also be dismiss-able. Probably beyond the scope for this PR, but if we can leave the log in place as a warning for now I think that's a step in the right direction.

@chaance I've implemented the --fix flag. Thanks for the feedback.

[SeanGroff]

I'd suggest that we keep the warning but allowing users to opt-in to automatic changes with --fix or something. That said, the warning should also be dismiss-able. Probably beyond the scope for this PR, but if we can leave the log in place as a warning for now I think that's a step in the right direction.

Anything else I can do to help progress this? @chaance

[github-actions]

🤖 Hello there,

We just published version v0.0.0-nightly-2e9c413-20230502 which involves this issue. If you'd like to take it for a test run please try it out and let us know what you think!

Thanks!

[github-actions]

🤖 Hello there,

We just published version 1.16.1-pre.0 which involves this issue. If you'd like to take it for a test run please try it out and let us know what you think!

Thanks!