bestguy / sveltestrap

Bootstrap 4 & 5 components for Svelte
https://sveltestrap.js.org
MIT License
1.3k stars 180 forks source link

Svelte 4 support #557

Open rallets opened 1 year ago

rallets commented 1 year ago

Hi, I'm wondering if you have any plans to support Svelte 4. Right now I need to force install sveltestrap via npm install --legacy-peer-deps. I see commit in main, does it means this package is still actively maintained? If so, any plans for a release soon-sh? Thank you

rallets commented 1 year ago

I played a bit, and I'm able to get typings support in VS Code, but in a ugly way, but I hope it can help you to fix easily in a better way. It looks like the .d.ts are not compatible with Svelte 4.

Once I found the pieces that was breaking the typings, I did add a rollup step in rollup.config.js that copied all the .d.ts in my app under App/typings/sveltestrap-d-ts.

The step is a copy with transforms using the rollup plugin rollup-plugin-copy:

copy({
                targets: [
                    {
                        src: 'node_modules/sveltestrap/**/*.d.ts',
                        dest: 'App/types/sveltestrap-d-ts',
                        transform: (contents, filename) => {
                            let ts = contents.toString();

                            ts = ts.replaceAll('SvelteComponentTyped', 'SvelteComponent');
                            ts = ts.replaceAll('export default class', 'export class');

                            ts = `declare module 'sveltestrap' {\r\n` + ts + `\r\n}`;

                            ts = ts.replaceAll(`Props,
  {},
  { default: {} }
> {}`, 'Props>{}');
                            ts = ts.replaceAll('Props, {}, {}>', 'Props>');

                            ts = ts.replaceAll(`}

export class`, `class?: string;
}

export class`);

                            return ts;
                        }
                    }
                ]
            }),

Unfortunately it's new-line sensible, so it could not work in a pipeline, but for a one-time transformation it could get the job done.

jonjes commented 1 year ago

@rallets I forced installed it, too. And then I noticed that the Modal component wasn't working properly. (The modal backdrop appears, but not the actual modal content.) Is it working for you? Otherwise, the rest of the sveltestrap components seem to be working ok with svelte 4.

rallets commented 1 year ago

@jonjes currently I don't use Models, only offcanvas, so I haven't checked them. If you provide a snippet/svelte component I can test it tomorrow.

bestguy commented 1 year ago

Hi, we are planning a Bootstrap 5.3.0 release very soon (this week?) after testing a pre release. Svelte 4 dropped on us as well unexpectedly.., so will likely need to deal with that after, but might be related to #560 and resolved with that as well.

bluepuma77 commented 11 months ago

svelte-file-dropzone just updated package.json to include v4:

  "peerDependencies": {
    "svelte": "^3.54.0 || ^4.0.0"
  },
kefahi commented 11 months ago

Any update on this? I'm happy to engage with testing the pre-release. Thanks!

jonjes commented 11 months ago

Hi @rallets Sorry, I just realized that you're waiting on me for an example. Here you go. https://svelte.dev/repl/38630548a4714255934cc8236f8ac7dc?version=4.1.1

It's using the Svelte REPL running svelte 4.1.1, and all I did was copy and paste the first Modal example from the Sveltestrap Modal page (https://sveltestrap.js.org/?path=/story/components--modals).

You can click the "Open Modal" button and see that the modal content doesn't appear. Only the gray background appears, and it locks the page because you can't clear it to close the invisible modal.

rallets commented 11 months ago

Hi @jonjes I confirm that it doesn't work for me either. Comparing with the bootstrap website modal live demo, it looks like the Modal doesn't set the "show" class properly in the modal div. So instead of "modal show" it has only "modal". Then also a display: block is not set in the same div element.

Until it is fixed, you can think to switch to Offcanvas, those are working for me. Basically you can get the same behavior, you just need to add some classes to mimic the modal "size" attributes. Just ping me if you need those.

bluepuma77 commented 11 months ago

I would really prefer a staged approach:

  1. Fix the dependencies version so sveltestrap works with current Svelte (package.json):

    "peerDependencies": {
    "svelte": "^3.54.0 || ^4.0.0"
    },
  2. Fix the few real issue that seem to exist

bestguy commented 11 months ago

Hey so in testing, another issue with Svelte 4 is we need to update all the TypeScript deps.. deprecation warnings and types complain about class props, etc: https://github.com/sveltejs/language-tools/blob/master/docs/preprocessors/typescript.md#im-getting-deprecation-warnings-for-sveltejsx--i-want-to-migrate-to-the-new-typings

To avoid breaking Svelte 3 apps, we'll probably have to do a major release to v6 and leave v5 for Svelte 3. I'm hoping this is a simple-ish search/replace fix, will look this weekend.

bestguy commented 11 months ago

Trying to support Svelte 3 & 4 is going to be a pain, so I think what I'll do is:

rallets commented 11 months ago

Hi @bestguy I just testet the latest release 5.11 and everything works fine for me.

About the Svelte 4 support, if you want I have already a list of things to change to get svelte 4 typings working better. Attached the modified code I use in my project, generated via a rollup task (it's not perfect but maybe it could help you to save some time) sveltestrap-d-ts.zip

The content of the zip file is:

As extra request, it will be nice to have an id attribute in the Accordion component, very useful for in-page anchors (f.ex. href="#accordion-id"). Now I need to wrap them with an extra div/section.

Summing up, for what I did experience currently it doesn't work:

  1. Modal (see previous messages)
  2. backdrop in Offcanvas I need to apply
    :global(.offcanvas.show ~ .offcanvas-backdrop) {
    opacity: 0.5;
    }

    not sure exactly what the problem is here.

Thanks for your work.

bestguy commented 11 months ago

This is great @rallets, thanks will review

benmccann commented 11 months ago

I suspect the other issue you'll run into is that the library is using an older version of svelte-jester, which isn't compatible with Svelte 4. That's going to be a decent sized upgrade on it's own. It can be handled separately, so I'd probably recommend doing that separately before the rest of the items to clear a blocker. I added some details in https://github.com/bestguy/sveltestrap/issues/568

kefahi commented 9 months ago

Thank you @rallets

Is it possible that you incorporate your changes for Svelte4 support in a PR?

I'm happy to test and contribute to that PR to make it as ready as possible.

rallets commented 9 months ago

@kefahi I would like to do it, but I am currently in parental leave and short of time. It's not easy to me to make a PR because the codebase is not typescript, so the amount of work is too much for me. I have attached a zip with all the needed changes I wrote in the last project. Let me know you are interested in getting an updated version. Also it looks the maintainer has not much time to work on it. I think Svelte team should try to invest a bit on this package, because bootstrap is very used around (basically all of the enterprise projects I worked in used bootstrap for something), it could help to increase the usage of Svelte, and this package requires only a bit of love to be really useful.

kefahi commented 9 months ago

Thank you @rallets And best wishes for your new born. Yes, please share with me your updated version, and I will work on producing a PR (and testing it). I fully agree that sveltestrap is very useful to many people using Svelte (I'm a big fan myself).

rallets commented 9 months ago

@kefahi tks, we has been blessed with a beautiful girl 🥰 Attached you can find the latest zip, not too much changes to the previous one. sveltestrap-d-ts.zip

I have also added my configs file, if you need them. It has all configs you could need, included svelte-jester:

The folder sveltestrap-d-ts contains the *.d.ts generated running the rollup copy pipeline.

The folder sveltestrap folder contains extra type definitions I need to add to the top of sveltestrap-d-ts to include other props not specified/not working in sveltestrap-d-ts. I know it's not valid typescript to have multiple definitions of the same type, but it just works™️ and I haven't found any other solutions to make it flexible enough to avoid to edit directly the sveltestrap .d.ts files.

Hope this helps. Just ping me if you wonder something 👍

jonjes commented 9 months ago

@rallets Congrats!!!

kefahi commented 9 months ago

My colleague @splimter has prepared a PR with the necessary changes ... additionally fixing the modal issue.

https://github.com/bestguy/sveltestrap/pull/574

I can reliably say that now Sveltestrap is working properly with Svelte4 for our usecase with no known issues.

@rallets @bestguy

jamiegau commented 8 months ago

When is the release version going to be made indicating Svelte 4 support? I am waiting on some other projects to support Svelte 4 and it's delayed due to this project not officially supporting Svelte 4 yet. So would appreciate an update to the main readme that it is now supported. Should this version is given a proper release number and dependency for svelte4.

kefahi commented 8 months ago

@jamiegau

May I suggest that you help with testing the above PR (Which provides Svelte4 support) to validate if it covers your usecase.

Please provide any notes / issues / feedback you may have. This will bring us closer to accomplishing the objective.

SoyaNyan commented 7 months ago

@kefahi Can I join with that testing? I want to migrate our project with Svelte4 and Sveltestrap. I hope next version of sveltestrap show up asap.

rallets commented 7 months ago

@kefahi It looks this project is not longer actively maintained, an it looks you guys have the capacity to bug fix and upgrade. have you thought to fork it and keep it rolling?