8000 fix: Create templates added types/node by brettearle · Pull Request #536 · sveltejs/cli · GitHub
[go: up one dir, main page]
Skip to content

fix: Create templates added types/node #536

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 2 commits into from
Closed

fix: Create templates added types/node #536

wants to merge 2 commits into from

Conversation

brettearle
Copy link
Contributor
@brettearle brettearle commented Apr 14, 2025 •
edited
Loading

Closes: #509

@changeset-bot changeset-bot
Copy link
changeset-bot bot commented Apr 14, 2025 •
edited
Loading

⚠️ No Changeset found

Latest commit: 0fecb9b

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

@brettearle brettearle mentioned this pull request Apr 14, 2025
Open
6 tasks
@brettearle brettearle changed the title Create templates: added types/node feat: Create templates added types/node Apr 14, 2025
@hyunbinseo
Copy link
Contributor

SvelteKit users could be using older versions of Node.js. v22 should not be hardcoded:

{ "engines": { "node": ">=18.13" } }

@brettearle
Copy link
Contributor Author
brettearle commented Apr 14, 2025 •
edited
Loading

Yeah that is fair, a post install script is bloat for created project.

Is there a simple way that you can think of, could just have ^18.13?

I'm thinking of adding a node detection function that can be used during the copy over of package.json from template to build out version of types/node. This might be overkill

EDIT: Ignore above, went with getting the version and updating in write common files

@brettearle brettearle marked this pull request as draft April 15, 2025 00:17
@brettearle brettearle marked this pull request as ready for review April 15, 2025 21:21
@brettearle brettearle changed the title feat: Create templates added types/node fix: Create templates added types/node Apr 15, 2025
@pkg-pr-new pkg.pr.new
Copy link
pkg-pr-new bot commented Apr 18, 2025

Open in StackBlitz

npx https://pkg.pr.new/sveltejs/cli/sv@536

npx https://pkg.pr.new/sveltejs/cli/svelte-migrate@536

commit: 0fecb9b

@manuel3108
Copy link
Member

Thanks for the PR! I'm not actually sure this is a safe way. Going to the node downloads it currently suggest downloading v23, but as far as I can see, there is no @types/node with version 23. I'm assuming similar things can happen for minor and patch versions as well.

Just having ^18.13 sounds like a safer way but relies on us adjusting that version to SK from time to time. But I would say that should be fine.

@AdrianGonz97
Copy link
Member

I'm not particularly convinced that this is something that we need add to the base create templates.

These type errors really only occur when native node modules are being used in the project.

In the case of the drizzle add-on issue, the fix would be to just add the @types/node package as a devDependency during its installation as we're using the process module to access env vars.

For the storybook issue, @types/node should really be added on storybook's end as we're simply running their initialization process via their CLI (or, if absolutely necessary, we could do it on our end too).

@brettearle
Copy link
Contributor Author
brettearle commented Apr 18, 2025 •
edited
Loading

Alright, I'll adjust the PR to just 18.13 on templates for now. Is that the plan? @manuel3108

Then we can update if needed to add-on by add-on basis?

@brettearle brettearle mentioned this pull request Apr 19, 2025
Closed
@brettearle brettearle closed this Apr 19, 2025
@AdrianGonz97 AdrianGonz97 mentioned this pull request Apr 20, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

add @types/node to npx sv create templates
4 participants
@brettearle @hyunbinseo @manuel3108 @AdrianGonz97
0