-
Notifications
You must be signed in to change notification settings - Fork 5
feat: setup cli project #23
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
Conversation
The latest updates on your projects. Learn more about Vercel for GitHub.
|
packages/mcp-stdio/package.json
Outdated
"type": "git", | ||
"url": "git+https://github.com/sveltejs/mcp.git" | ||
}, | ||
"author": "Author Name <author.name@mail.com>", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
needs an update?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ugh yeah i meant to remove it
packages/mcp-stdio/package.json
Outdated
"files": [ | ||
"dist" | ||
], | ||
"main": "./dist/index.js", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd prefer exports map over main
main and bin being the same seems odd as well. does this export anything or is it cli only? maybe just remove main and not have an exports map?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Uh I was sure i removed it...fixing now
packages/mcp-stdio/tsdown.config.ts
Outdated
platform: 'node', | ||
define: { | ||
// some eslint-plugin-svelte code expects __filename to exists but in an ESM environment it does not. | ||
__filename: '""', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
doesn't that risk breaking stuff if something actually uses __filename
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I checked and it doesn't seem to be used by anything we use really...maybe we can define it with the actual current filename using import.meta.url
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually scratch that we can replace it with import.meta.filename
// some eslint-plugin-svelte code expects __filename to exists but in an ESM environment it does not. | ||
__filename: '""', | ||
}, | ||
// we need eslint at runtime but the bundler doesn't bundle `require`'s which `eslint-plugin-svelte` uses to require |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
where does eslint-plugin-svelte come from? i don't see it in dependencies
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a dependency of @sveltejs/mcp-server
This sets up the project for the stdio transport...is still missing the db setup, but I'm actually questioning if we should bundle the db in the cli instead of downloading it when the user runs
npx @sveltejs/mcp setup
, especially because we would find the right place where to put the db, have conditional configurations for drizzle for minimal gains (we can publish a new version every now and then with the updated DB to get the cli users the updated documentation, we might even automate it).There's some bit of weirdness going on, specifically, we need to install
eslint
as a dependency because the bundler doesn't touch therequire('eslint/use-at-your-own-risk');
ineslint-plugin-svelte
...by having it as a dependency, we guarantee thateslint
will be available at runtime and can be installed.It also makes the actual file a tad lighter (currently sits at 12MB, it was 17MB 🤯).