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

Make CLI up-to-date #2007

Open
wants to merge 20 commits into
base: main
Choose a base branch
from
Open

Make CLI up-to-date #2007

wants to merge 20 commits into from

Conversation

ahkhanjani
Copy link
Contributor

Closes #

✅ Checklist

  • I have followed every step in the contributing guide (updated 2022-10-06).
  • The PR title follows the convention we established conventional-commit
  • I performed a functional test on my final commit

Changelog

[Short description of what has changed]


Screenshots

[Screenshots]

💯

Copy link

changeset-bot bot commented Oct 31, 2024

⚠️ No Changeset found

Latest commit: afca173

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

Copy link

vercel bot commented Oct 31, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
create-t3-app ✅ Ready (Inspect) Visit Preview 💬 Add feedback Nov 6, 2024 10:03pm

Copy link

vercel bot commented Oct 31, 2024

@ahkhanjani is attempting to deploy a commit to the t3-oss Team on Vercel.

A member of the Team first needs to authorize it.

@ahkhanjani
Copy link
Contributor Author

The new ESLint installer could works by:

  1. Having an imports (imported plugins) array that can dynamically change based on the selected packages (currently drizzle only).
  2. Marking the imported plugins with "%%PLUGIN_NAME%%" in the JSON object and stripping "%% and %%" from them. The marker could be anything. I chose something that wasn't likely to be used.
  3. The plugins section is also dynamic and must use the marker.
  4. Formatting the end result with prettier.

Note: At the moment we're not reading the CLI's generated Prettier config. We could do that and we'd get a more accurate formatting. I'm not sure if we could apply the Prettier plugins though (e.g. @ianvs/prettier-plugin-sort-imports) so we'll have to handle the order of the imports array manually.

@ahkhanjani
Copy link
Contributor Author

@juliusmarminge

@ahkhanjani
Copy link
Contributor Author

I don't understand the linting errors we're getting

cli/package.json Outdated Show resolved Hide resolved
cli/package.json Outdated Show resolved Hide resolved
cli/template/base/package.json Outdated Show resolved Hide resolved
cli/template/base/tsconfig.json Outdated Show resolved Hide resolved
@ahkhanjani
Copy link
Contributor Author

@juliusmarminge drizzle needs updating. Any breaking changes I need to know of?

@ahkhanjani
Copy link
Contributor Author

I think we should upgrade tsup as well while we're at it

@ahkhanjani
Copy link
Contributor Author

I think we should upgrade tsup as well while we're at it

The only breaking changes between major versions 6 to 7 to 8 seems to be node version support drops so I'll just bump the version.

@ahkhanjani
Copy link
Contributor Author

@juliusmarminge I think we're only pending on drizzle.

since we're writing to fs without formatting on error, we should be as good looking as possible by default
@ahkhanjani ahkhanjani changed the title cli: Next.js 15 Make CLI up-to-date Nov 8, 2024
@ahkhanjani ahkhanjani mentioned this pull request Nov 9, 2024
3 tasks
@realslimsutton
Copy link

realslimsutton commented Nov 30, 2024

@juliusmarminge I think we're only pending on drizzle.

Hey @ahkhanjani, thank you for your work on this! I'm just curious as to whether you know if this is on Drizzle's roadmap? I saw the issue you linked to their repo, but it seems that hasn't had any activity since.

@ahkhanjani
Copy link
Contributor Author

Hey @ahkhanjani, thank you for your work on this!

I appreciate your words.

I'm just curious as to whether you know if this is on Drizzle's roadmap? I saw the issue you linked to their repo, but it seems that hasn't had any activity since.

I don't remember the issue you're mentioning. Can you send the link here?

@ahkhanjani
Copy link
Contributor Author

@juliusmarminge Why are we not merging btw?

@realslimsutton
Copy link

realslimsutton commented Dec 1, 2024

Hey @ahkhanjani, thank you for your work on this!

I appreciate your words.

I'm just curious as to whether you know if this is on Drizzle's roadmap? I saw the issue you linked to their repo, but it seems that hasn't had any activity since.

I don't remember the issue you're mentioning. Can you send the link here?

Oh, apologies, I thought you had linked the issue but I had actually found it myself. The issue is drizzle-team/drizzle-orm#2491

There's an issue with Drizzle about supporting the new flat config, which forgive me if I'm wrong since I've not looked into the ESLint 8 to 9 migration, is what I thought you meant about waiting for Drizzle?

(quick note to also mention that I've only just seen your later commit to update Drizzle which could be what you had meant you was waiting for)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants