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 4 support #557

Open
rallets opened this issue Jun 26, 2023 · 24 comments
Open

Svelte 4 support #557

rallets opened this issue Jun 26, 2023 · 24 comments

Comments

@rallets
Copy link

rallets commented Jun 26, 2023

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
Copy link
Author

rallets commented Jun 27, 2023

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
Copy link

jonjes commented Jul 5, 2023

@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
Copy link
Author

rallets commented Jul 5, 2023

@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
Copy link
Owner

bestguy commented Jul 7, 2023

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
Copy link

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

  "peerDependencies": {
    "svelte": "^3.54.0 || ^4.0.0"
  },

@kefahi
Copy link

kefahi commented Jul 21, 2023

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

@jonjes
Copy link

jonjes commented Jul 21, 2023

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
Copy link
Author

rallets commented Jul 26, 2023

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
Copy link

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"
  },
  1. Fix the few real issue that seem to exist

@bestguy
Copy link
Owner

bestguy commented Aug 4, 2023

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
Copy link
Owner

bestguy commented Aug 5, 2023

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

  • Cut a minor release now for Bootstrap 5.3 for Svelte 3 (so like v5.11.0), and leave peer dep issues as is.
  • Fix the peer deps by updating minimum to Svelte 4, correct typings, then release a major version (v6.0)

@rallets
Copy link
Author

rallets commented Aug 5, 2023

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:

  • the rollup task I use to transform your *.d.ts code. You can use it to take a look there to see what/how you need to change your source code for the typings
  • sveltestrap-d-ts: your typings, from your latest release 5.11.0, transformed by my rollup task
  • sveltestrap: extra typings, to address missing attributes from your typings. I use only few components, so the list is not exhaustive.

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
Copy link
Owner

bestguy commented Aug 6, 2023

This is great @rallets, thanks will review

@benmccann
Copy link
Contributor

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 #568

@kefahi
Copy link

kefahi commented Sep 22, 2023

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
Copy link
Author

rallets commented Sep 22, 2023

@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
Copy link

kefahi commented Sep 22, 2023

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
Copy link
Author

rallets commented Sep 23, 2023

@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:

  • .eslintrc.json => eslint config (not needed)
  • jest.config.json => svelte-jester typescript config
  • package.json => all my packages (in reality part of them, I remove other not relevant stuff)
  • rollup-task.ts => one (of many) task I use in my rollup pipeline to build Svelte code
  • rollup.config.js => complete rollup pipeline, as reference if you wonder something
  • tsconfig.json => tsconfig (I think you don't need resolveJsonModule and the include should be customized)
  • tsconfig.spec.json => tsconfig for unit-tests/Jest (as suggested as best practice also in their documentation, but not needed)

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
Copy link

jonjes commented Sep 29, 2023

@rallets Congrats!!!

@kefahi
Copy link

kefahi commented Oct 9, 2023

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

#574

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

@rallets @bestguy

@jamiegau
Copy link

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
Copy link

kefahi commented Oct 13, 2023

@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
Copy link

@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
Copy link
Author

rallets commented Nov 11, 2023

@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?

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

No branches or pull requests

8 participants