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

create-svelte: fix global.d.ts rename, Windows build issue, missing adapter-node #1095

Merged
merged 1 commit into from
Apr 18, 2021

Conversation

GrygrFlzr
Copy link
Member

@GrygrFlzr GrygrFlzr commented Apr 18, 2021

  • Fixes Cannot find module '@sveltejs/adapter-node' #1093
  • Fix global.d.ts getting renamed to global.d.js
  • Fix create-svelte build script on Windows
  • Make svelte.config.cjs adapter-agnostic
  • Remove default adapter-node from package.json
  • Version updates for dependencies

@vercel
Copy link

vercel bot commented Apr 18, 2021

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/sveltejs/kit-demo/5nGJvmnv8NFREA54ye9dptYjRy8m
✅ Preview: https://kit-demo-git-cli-fixes-sveltejs1.vercel.app

@dummdidumm
Copy link
Member

The version updates shouldn't be needed because the ^ means "install the latest version", which would be what you changed them to. Bumping the minor version is ok for me though.
What do the changes to make it adapter-agnostic mean for the user? Does he need to set an ADAPTER and OPTIONS env var now himself?

@GrygrFlzr
Copy link
Member Author

What do the changes to make it adapter-agnostic mean for the user? Does he need to set an ADAPTER and OPTIONS env var now himself?

Yes, I presumed that was @Rich-Harris' intention, I just moved over the demo app's svelte.config.cjs to the +typescript and -typescript ones, which were overwriting the template svelte.config.cjs in the first place. I'm not sure if this is actually better than just defining it in the svelte.config.cjs directly, most users probably only need one.

@Rich-Harris
Copy link
Member

It wasn't my intention :) The demo app's svelte.config.cjs is like that so that we can deploy it to different targets by installing all the adapters into the package.json (which is replaced with package.template.json) and using the env var. When a new project is created, it should replace our internal config with the public config in +typescript or -typescript

@GrygrFlzr
Copy link
Member Author

Alright, I'll revert the config-related changes then.

What should happen with the package.json in the default template? It's causing pnpm -r build to pick it up and fail to build.

@Rich-Harris
Copy link
Member

it shouldn't be, since #1088 — I removed all the pnpm -r build occurrences in favour of more targeted filtering

@GrygrFlzr GrygrFlzr changed the title create-svelte config changes, dependency updates, and default adapter removal create-svelte: fix global.d.ts rename, Windows build issue, missing adapter-node Apr 18, 2021
@GrygrFlzr
Copy link
Member Author

Gotcha, I'll leave the package.json in then. Removed the unnecessary changes.

@Rich-Harris Rich-Harris merged commit a84cb88 into master Apr 18, 2021
@Rich-Harris Rich-Harris deleted the cli-fixes branch April 18, 2021 11:58
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.

Cannot find module '@sveltejs/adapter-node'
3 participants