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

Add svelte exports field to package json when running package command #1959

Closed
dummdidumm opened this issue Jul 19, 2021 · 6 comments · Fixed by #2431
Closed

Add svelte exports field to package json when running package command #1959

dummdidumm opened this issue Jul 19, 2021 · 6 comments · Fixed by #2431
Labels
p1-important SvelteKit cannot be used by a large number of people, basic functionality is missing, etc. pkg:svelte-package Issues related to svelte-package

Comments

@dummdidumm
Copy link
Member

Describe the problem

SvelteKit currently has a heuristic in place which tells Vite which packages are Svelte packages (for noExternal stuff) by looking up the package.json content and seeing if there's a svelte property.

The svelte-kit package command does not currently set this field.

Describe the proposed solution

Use the same logic that is used for the main entry to produce a svelte entry.

Alternatives considered

A workaround for now is for users to specify this field in their package.json themselves as it's copied over as of lately.

Importance

would make my life easier

Additional Information

The svelte key is likely more of a historical thing at this point, and deserves some more thought regarding the new developments, but for now the quick fix is to just add this field

@dummdidumm dummdidumm added the pkg:svelte-package Issues related to svelte-package label Jul 19, 2021
@benmccann
Copy link
Member

benmccann commented Jul 20, 2021

From Rich:
originally the reason for pkg.svelte was, IIRC, so that a package could expose uncompiled files alongside a standalone dist file, but the latter use case is rare enough that i'm not sure it's worthwhile. and it's very blunt — it's package-level, whereas if we need it at all it should probably be a new key on pkg.exports alongside import and require
we are, however, using it inside Kit to automatically add Svelte component libraries to ssr.noExternal, so to answer @dummdidumm's question above, yes we probably should be adding it to those package.json files. but maybe it ought to be a boolean instead of an entry point? idk. it's all got a bit messy; we designed it in ~2017 without sufficient foresight
anyway i gtg but this probably warrants full discussion

@dummdidumm
Copy link
Member Author

I just tested this out and it seems Vite is able to deal with a Svelte package packaged through the package without the svelte field, maybe because of "type": "module" and the exports field.
So I'm not 100% sure at this point how to proceed here. Should we deprecate this convention now that it seems to work?

On an unrelated note: I noticed that Vite was not able to deal with exports besides the default export, so deep exports from exports like "./foo": "./Foo.svelte" did not work. I don't have time to look into this right now, just leaving this here for visibility.

@dummdidumm
Copy link
Member Author

Seems like there's a case where an error occurs: #1856 . So this thing should likely be added somehow.

@mattjennings
Copy link

mattjennings commented Aug 2, 2021

Without pkg.svelte being defined, the library has to be added to SvelteKit projects as a regular dependency (i.e not a devDependency) otherwise they will get the Must use import to load ES Module error. I assume, because find_svelte_packages fails to detect them

function find_svelte_packages(cwd) {
const pkg_file = path.join(cwd, 'package.json');
if (!fs.existsSync(pkg_file)) return [];
const pkg = JSON.parse(fs.readFileSync(pkg_file, 'utf8'));
const deps = [...Object.keys(pkg.dependencies || {}), ...Object.keys(pkg.devDependencies || {})];
return deps.filter((dep) => {
const dep_pkg_file = path.join(cwd, 'node_modules', dep, 'package.json');
if (!fs.existsSync(dep_pkg_file)) return false;
const dep_pkg = JSON.parse(fs.readFileSync(dep_pkg_file, 'utf-8'));
return !!dep_pkg.svelte;
});
}

and I guess regular dependencies automatically get included under vite.ssr.noExternal?

Would it make sense to also check pkg.exports for any entrypoints with a .svelte extension?

@benmccann
Copy link
Member

That might get fixed by vitejs/vite#4450

@benmccann benmccann added the p1-important SvelteKit cannot be used by a large number of people, basic functionality is missing, etc. label Aug 4, 2021
@ignatiusmb
Copy link
Member

deep exports from exports like "./foo": "./Foo.svelte" did not work

I might've reduced my lifespan when I first encountered this and trying to figure out what's wrong back in the early days before version .50 or lower. I found out that we also needed to specify the deep exports with an object. To this day, I'm still not sure why exactly, but this would resolve the deep exports when imported (I'm pretty sure we still need to include them in the noExternal list)

"exports": {
  "./foo": {
    "import": "./foo/index.js"
  }
}

This should solve our deep exports problem, but I hope those more familiar with Vite could take a look and possibly understand what's going on, enough to solve it on their side.

P.S. It's been months since then so I don't know if something has changed with this behavior, almost certain it's still the same

dummdidumm pushed a commit that referenced this issue Sep 15, 2021
Several heuristics in Kit/vite-plugin-svelte to tell Vite to mark Svelte packages rely on the "svelte" property. Vite/Rollup/Webpack plugin can all deal with it.

Fixes #1959
dummdidumm added a commit that referenced this issue Sep 16, 2021
Several heuristics in Kit/vite-plugin-svelte to tell Vite to mark Svelte packages rely on the "svelte" property. Vite/Rollup/Webpack plugin can all deal with it.

Fixes #1959

Co-authored-by: Bjorn Lu <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
p1-important SvelteKit cannot be used by a large number of people, basic functionality is missing, etc. pkg:svelte-package Issues related to svelte-package
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants