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

svelte-kit package #1499

Merged
merged 14 commits into from
May 24, 2021
Merged

svelte-kit package #1499

merged 14 commits into from
May 24, 2021

Conversation

Rich-Harris
Copy link
Member

@Rich-Harris Rich-Harris commented May 20, 2021

This is an early and incomplete attempt at svelte-kit package (#518). It adds a command that packages up the contents of the src/lib folder for publishing to npm, and some corresponding config:

// svelte.config.js
export default {
  kit: {
    package: {
      // default values are shown
      dir: 'package',
      exports: {
        include: ['**'],
        exclude: []
      },
      files: {
        include: ['**'],
        exclude: []
      }
    }
  }
};

Specifically, it:

  • copies all the files in src/lib that are specified by config.kit.package.files
  • preprocesses any Svelte files
  • transforms TypeScript files (TODO)
  • adds a package.json that copies most values from the root package.json, but also adds an "exports" map that includes all the files specified by config.kit.package.exports

It doesn't apply other transforms, either Vite built-in transforms or those specified by user plugins. I'm open to persuasion on that front but I think it's better to keep everything as simple as possible.

I'd like it to emit .d.ts files for .ts files and, if possible (if it even makes sense) .svelte files that contain TypeScript. Not familiar enough with the TypeScript API to know how to go about that.

The expected publishing workflow looks something like this:

npx svelte-kit package && npm publish package

Other possibilities mentioned in #518:

  • exclude files with a _ prefix from exports automatically
  • src/lib/Foo/index.svelte -> import Foo from 'my-lib/Foo'
  • expose compiled components for consumption as regular modules
  • expose formats other than ESM

Before submitting the PR, please make sure you do the following

  • It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs
  • This message body should clearly illustrate what problems it solves.
  • Ideally, include a test that fails without this PR but passes with it.

Tests

  • Run the tests with pnpm test and lint the project with pnpm lint

Changesets

  • If your PR makes a change that should be noted in one or more packages' changelogs, generate a changeset by running pnpx changeset and following the prompts

@dummdidumm
Copy link
Member

dummdidumm commented May 20, 2021

Re TypeScript declarations:
https://github.com/micha-lmxt/svelte-types-writer
And
https://github.com/firefish5000/svelte2dts
Both implement something like this for Svelte files. Basically they transform Svelte (with unpreprocessed TypeScript) to tsx using svelte2tsx (part of language tools) and run tsc on it to emit declaration files. (You can ignore the part in their readme about not placing DTS next to the Svelte file, this is now possible)
I think something like this (generating this DTS files) would make sense to extract into a separate official package once we are happy with the results.

@Rich-Harris
Copy link
Member Author

Oh, that's good to know. svelte2dts looks particularly promising, especially if it can be coaxed into generating declaration files for Svelte components with JSDoc types. May take a stab at integrating that later if I get time, though I suppose an alternative could be to just support vanilla JS for now and worry about types in a follow-up

@benmccann
Copy link
Member

We'll have to add some docs for this as well

@Rich-Harris
Copy link
Member Author

I had a crack at integrating svelte2dts but got nowhere, so in the interests of shipping the MVP version of this command I think it probably makes sense to tackle types in a follow-up.

@dummdidumm @halfnelson I did notice that svelte2tsx returns an object with exportedNames and events properties. I didn't study it too closely but it almost feels like we could generate .d.ts files from that information? I'm glossing over slots and inferred types here, and probably many other things, but is there a future in which we can generate declarations directly from svelte2tsx or have I grossly underestimated the problem?

@dummdidumm
Copy link
Member

I don't see a near future where this is happening, precisely because of inferred types. I also think that it's better to just let TypeScript handle this stuff, especially since the wrapper to get this working is that small (the svelte2dts code is only a couple of hundred lines).
Happy to take a crack at this after this is merged.

@Rich-Harris Rich-Harris marked this pull request as ready for review May 21, 2021 20:12
@ryanatkn
Copy link
Contributor

ryanatkn commented May 21, 2021

For generating Svelte type definitions there's also https://github.com/IBM/sveld (no lang="ts" support yet if ever) and svelte-type-generator https://github.com/material-svelte/material-svelte/tree/main/tools/svelte-type-generator (uses svelte2tsx)

@dummdidumm
Copy link
Member

Two things that come to my mind:

  • what about css files that are referenced?
  • when writing a package I often want to define the public API myself. That means I have some index.js/ts from which I re-export the public stuff. This makes for both better imports (only one location) and I can also freely reorganize my files internally without risking breaking changes. Is that possible with the current solution?

@Rich-Harris
Copy link
Member Author

what about css files that are referenced?

any non .svelte files are copied across as-is, so a module can do import './foo.css' and it will continue to work. they will also have exports generated for them by default, so a consumer can do import 'some-lib/base.css' or whatever

when writing a package I often want to define the public API myself

yes, any src/lib/index.js or src/lib/index.svelte becomes the main entry point. you can disable all other entry points by doing this...

// svelte.config.js
export default {
  kit: {
    package: {
      exports: ['index.js']
    }
  }
};

...and defining the API in src/lib/index.js. Consumers can then do this:

import { X, Y, Z } from 'your-lib';

@Rich-Harris Rich-Harris merged commit 6372690 into master May 24, 2021
@Rich-Harris Rich-Harris deleted the svelte-kit-package branch May 24, 2021 15:39
andyburke pushed a commit to andyburke-forks/kit that referenced this pull request May 25, 2021
* first stab at svelte-kit package command

* include/exclude files as well

* make typescript happy

* tidy up

* improve error message

* exclude underscore-prefixed files from exports by default

* move files

* add docs

* changeset

* gitignore package directory

* update lockfile, who knows why

* update unit tests

* copy over readme
@TheComputerM
Copy link
Contributor

For generating Svelte type definitions there's also https://github.com/IBM/sveld (no lang="ts" support yet if ever) and svelte-type-generator https://github.com/material-svelte/material-svelte/tree/main/tools/svelte-type-generator (uses svelte2tsx)

Just a warning, pls try to not use sveld as it cannot parse custom css such as sass and cannot detect props which are exported as export {localName as name}. If you need jsdoc-like syntax for generating declaration files, please check out what I made. But I think it is better to provide an official way to generate types instead of looking into third part ones.

@@ -1,5 +1,4 @@
.DS_Store
node_modules
/.svelte-kit
/build
Copy link
Member

Choose a reason for hiding this comment

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

@Rich-Harris did you mean to remove both /build and /functions here?

@manuel3108
Copy link
Contributor

Any chance we can get a new kit release for this? #1543
That would create the possibility to easily test it out.

sidharthv96 added a commit to sidharthv96/kit that referenced this pull request May 29, 2021
* 'master' of https://github.com/sveltejs/kit:
  Version Packages (next) (sveltejs#1543)
  type fixes for adapter-node and adapter-static (sveltejs#1578)
  Upgrade to Vite 2.3.3 (sveltejs#1580)
  fix: improve getRawBody parsing & handle error(s) (sveltejs#1528)
  create-svelte: add svelte-check for TS (sveltejs#1556)
  pass validated svelte config to adapters (sveltejs#1559)
  types: group related and reduce potential inconsistencies (sveltejs#1539)
  Use sveltekit tag on StackOverflow (sveltejs#1558)
  Fix create-svelte build-template script (sveltejs#1555)
  Remove err param from Polka .listen() callback (sveltejs#1550)
  bump: polka and sirv versions (sveltejs#1548)
  svelte-kit package (sveltejs#1499)
@janosh
Copy link
Contributor

janosh commented Jul 6, 2021

How is this supposed to work for single-component packages? If I add an index file that simply reexports the Svelte component,

// src/lib/index.js
export { default } from './MyComponent.svelte'

and then install the package and try

import MyComponent from 'my-package'

I get

Must use import to load ES Module: my-package/index.js
require() of ES modules is not supported.
require() of my-package/index.js from repo/node_modules/vite/dist/node/chunks/dep-0ed4fbc0.js is an ES module file as it is a .js file whose nearest parent package.json contains "type": "module" which defines all .js files in that package scope as ES modules.
Instead rename index.js to end in .cjs, change the requiring code to use import(), or remove "type": "module" from my-package/package.json.

The easiest solution to this in the past was to use the "svelte" field in package.json but sveltekit package does not copy that field into its output...

@benmccann
Copy link
Member

I'm not super familiar with svelte-kit package, but I think you can just have Index.svelte and don't need an index.js if all you're doing is exporting a single component

@janosh
Copy link
Contributor

janosh commented Jul 8, 2021

I don't think using a Index.svelte would allow me to import as

import MyComponent from 'my-package'

Would't I have to do this?

import MyComponent from 'my-package/Index.svelte'

If so, I'd say it's better to keep MyComponent.svelte and import it as

import MyComponent from 'my-package/MyComponent.svelte'

@anthonylegoas
Copy link

  • expose compiled components for consumption as regular modules

Is there any workaround to do it until it's possible with svelte-kit package ?

@anthonylegoas
Copy link

  • expose compiled components for consumption as regular modules

Is there any workaround to do it until it's possible with svelte-kit package ?

Finally I did it with rollup, in addition to vite in my svelte-kit project.

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.

8 participants