-
Notifications
You must be signed in to change notification settings - Fork 45
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
Sveltekit mega change, for better robustness #107
Conversation
…cture, and change dir structure for lib packaging Still need to have better typing and to put the sveltekit example in the routes.
…book build instruction
Still have a problem on static content in routes.
I think Netlify fails because it uses old Node.js version. See in https://app.netlify.com/sites/svelte-file-dropzone/deploys/62f24201903fd400087f9dc3#L64-L84:
|
Wow..this is awesome. Thanks for creating PR. I will look into this before Sunday.
Will see if any upgrade can help. |
Notice that you must run |
wait. Why do we want to move an example to main parts of repo? This makes our repo look more of App oriented rather library. |
Also don't see much of advantage with vite over rollup except probably a friendly config. Vite internally uses rollup for prod build. |
Renaming to ts means REPL support is gone. I would rather prefer typedefs. Check dasDaniel/svelte-table#97 . They reverted from typescript recently. |
I feel like I need to explain the full story that I've seen trough some Rich Haris's(the Svelte inventor) YouTube video. I can't find this YouTube sadly, so I have to say it briefly:
All I said is mentioned (with a too short description) in SvelteKit packaging docs. The example library you mentioned is just another proof that sadly SvelteKit packaging is purely documented and used. They fixed the problem that everyone have, but still most maintainers don't know the solution exists already. Another library that actually do the packaging well(I'm also developing this library, but it was like this before I entered) is Svelte Splitpanes. |
I used Vite for two reasons:
|
Hope I'm not opening a can of worms but just wanted to point out that all of this is true for any compilation step, there's nothing particularly special about sveltekit's packager compared to just running rollup/vite on components, and I find it a slightly odd choice for a small focussed library like this (as opposed to, for eg, a design system) |
Ok. If the PR spitting typedefs. I am fine with that. But wanted to makesure it works in REPL, Svelte 3 & Sveltekit without any issues. As @madeleineostoja pointed out, there should be a reason why REPL may have not worked in
It's not just about Rich recommendation. It makes sense in his suggestion if we don't have storybook. But we have good story book setup which help us easily setup and test. Anyways let me play around this branch and take a call.
Looks like you are the contributor for same there as well 😄 . Nothing wrong just saying. I tried exploring much more popular svelte libs. Didn't find any following same. Anyways, let me explore advantages with the changes you made and get back to you in couple of days. |
The regular Svelte pipeline by rollup/sveltekit/whatever, is to:
When you run SvelteKit packaging, it do only step 1, and additionally emit typescript def files. The resulted prepared to be published package is at the "package" direcotry. The idea is that step 1 isn't universally agreeable. Some like typescript, some like scss, some like postcss, some like to use some other high-end custom preprocessing features, and so on. When the library user(e.g. REPL) consumes the library, he may still do the preprocessing(as step 1, and then also step 2 and 3) by svelte to your component - but there should not be any noticable effect since they are already in their "pure" form. |
I think that the reason is simple - this library is "old" from 2019, back when there were no SvelteKit packaging (and even maybe not SvelteKit at all).
Notice that in this PR I didn't mix the SvelteKit code with storybook, not in directory structure nor in npm script (
I mentioned before the disclaimer briefly - the maintainer had already followed Rich's suggestion of SvelteKit packaging and the example-in-route-dir since the start of this project, before I had even seen it, so although I'm suspected here, I'm objective though😂
There's a new player in town 🤭: https://github.com/chanced/filedrop-svelte But seriously, see this popular YouTube video and how the new method becomes more and more hot. Since he always try to simplify things, he doesn't explain the advantages using this method. |
BTW I recommend seeing the newly published library leveluptuts/bookit, though it's still experimental. It is presented in the following youtube video . |
Found the video with the misleading name! |
Hi! Any update? |
Hey @Tal500 Sorry. Was occupied with personal stuff. Will be looking into this today. |
I like their folder structure. All component files are placed in package folder. |
I am thinking we should support him by using bookit in our repo as well. @Tal500 I am moving your code to |
@Tal500 Also note that I might have to remove pnpm usage as I don't use it any repos. It's great but just for one repo can't have another node modules cache folder. |
Hi again!
As far as I know, PNPM is 100% compatible with NPM, except:
That means you need to choose between PNPM/NPM in the repo level, but not in the dependency graph level. What I mean by the latter, is that a project A can be dependent on the NPM library(notice: Both NPM and PNPM generates 100% NPM library), no matter whether A or B are similar or different by their choices between NPM and PNPM. The reason for that is that theinternal lockfile and the internal As an example, sadly due to CI pipeline, I'm using NPM in my website, but some of my dependencies uses PNPM. I couldn't tell this by looking on the final NPM library, but only by looking on their repos. |
My point is about global cache directories. pnpm AFAIK manages it's own like npm and yarn. |
That's true. |
it's not for global use. cache directories are where you have a copy stored. When you do npm install, cache directory is where npm/pnpm searches first later downloads from internet registries. In pnpm case i think they have specific word called pnpm store directory to which actually links are created. |
This is a mega PR for this project, to be more in the "Svelte(Kit) way".
Briefly, what I have done:
pnpm package
, only the "lib" directory is being compiled and exported.Additionally,I have tested importing this library in my own Sapper project, and it works great as it should (meaning that it only depends on Svelte, no need to use SvelteKit for using the library - as a principal).
Please see the updated
README.md
for the new instructions.One thing that isn't written there - you may check Svelte&TS typing by the command
pnpm check
. There is one unresolved issue, that must not be "fixed", otherwise it will break legacy support:I'm opening for comments, though it was a hard and a huge work.
fixes #89
fixes #73
maybe also #60
I'm sure it'll solve #45 , didn't test it however.
closes #106