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

docs(next): Refactor PM comps to use package-manager-detector #1491

Merged
merged 7 commits into from
Nov 18, 2024

Conversation

ieedan
Copy link
Contributor

@ieedan ieedan commented Nov 15, 2024

Fixes #1474

  • Adds run dev server once to installation instruction
  • Adds <PMRun/> component
  • Refactors the pm-block component with package-manager-detector so adding new commands is easier.

Copy link

changeset-bot bot commented Nov 15, 2024

⚠️ No Changeset found

Latest commit: 30ce39e

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

@ieedan
Copy link
Contributor Author

ieedan commented Nov 15, 2024

package-manager-detector won't deploy because of cloudflare even though we aren't using that part of the functionality :(

@ieedan
Copy link
Contributor Author

ieedan commented Nov 15, 2024

I almost want to open up an issue on package-manager-detector to see if they would be open to publishing another package with just the resolveCommand functionality as it is really useful for the application we are using it for now and we surely aren't and won't be the only ones to experience this.

edit:
Maybe not another package but I think (or hope) that if they export types we can just import from /types and /commands without importing from index.ts and maybe that would solve this problem.

@AdrianGonz97
Copy link
Collaborator

I don't think this will be necessary now that #1487 is merged. svelte-kit sync ensures that the .svelte-kit directory is generated. If it's missing in the project, the CLI runs the command to generate it for them

@ieedan
Copy link
Contributor Author

ieedan commented Nov 15, 2024

I don't think this will be necessary now that #1487 is merged. svelte-kit sync ensures that the .svelte-kit directory is generated. If it's missing in the project, the CLI runs the command to generate it for them

I think you are right let me try and reproduce their issue without the new steps before closing it!

@ieedan
Copy link
Contributor Author

ieedan commented Nov 15, 2024

Yeah I can't reproduce it with the latest version.

I still think this has some good changes to the PM components but unless something changes with package-manager-detector we can't really use them.

Copy link
Contributor

github-actions bot commented Nov 17, 2024

built with Refined Cloudflare Pages Action

⚡ Cloudflare Pages Deployment

Name Status Preview Last Commit
shadcn-svelte ✅ Ready (View Log) Visit Preview 30ce39e

@ieedan ieedan changed the title docs(next): Add run dev server once to SvelteKit installation instruction docs(next): Refactor PM components to use package-manager-detector Nov 17, 2024
@ieedan
Copy link
Contributor Author

ieedan commented Nov 17, 2024

Changing this just to refactor the PM component.

All I had to do to make it work was use type only imports from the root of the project so that it doesn't load node. And then import resolveCommand from /commands.

@huntabyte
Copy link
Owner

CleanShot.2024-11-17.at.13.41.44.mp4

Is this actually how scripts are executed with bun or should it be bunx?

@ieedan
Copy link
Contributor Author

ieedan commented Nov 17, 2024

CleanShot.2024-11-17.at.13.41.44.mp4

Is this actually how scripts are executed with bun or should it be bunx?

Yeah you're right I'm either using resolve command wrong or there's a bug I can hopefully find the time to take a look today if not tomorrow

@ieedan
Copy link
Contributor Author

ieedan commented Nov 17, 2024

Okay so actually bunx is a shorthand for bun x

https://bun.sh/docs/cli/bunx

But I think pretty much everyone just says bunx so maybe I should just make an exception for bun and that command?

@huntabyte
Copy link
Owner

@AdrianGonz97 what are your feelings about this lol

@huntabyte
Copy link
Owner

We're going to keep it as bun x as it is the actual command rather than an alias.

Copy link
Owner

@huntabyte huntabyte left a comment

Choose a reason for hiding this comment

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

Thank you!

@huntabyte huntabyte changed the title docs(next): Refactor PM components to use package-manager-detector docs(next): Refactor PM comps to use package-manager-detector Nov 18, 2024
@huntabyte huntabyte merged commit 103f7d5 into huntabyte:next Nov 18, 2024
4 checks passed
@ieedan ieedan deleted the installation-instruction branch November 18, 2024 19:14
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