Fixs for svelte4 by splimter · Pull Request #574 · bestguy/sveltestrap

splimter · 2023-10-09T09:52:32Z

Fixing Modal
Fixing the .d.ts files

LoopControl · 2023-10-11T01:48:33Z

When I do npm install 'git+https://github.com/bestguy/sveltestrap.git#pull/574/head' to install from this pull request, I'm getting the following error:

npm ERR! Found: svelte@4.2.1
npm ERR! node_modules/svelte
npm ERR!   dev svelte@"^4.0.5" from the root project
npm ERR! 
npm ERR! Could not resolve dependency:
npm ERR! peer svelte@"^3.53.1" from sveltestrap@5.11.2
npm ERR! node_modules/sveltestrap
npm ERR!   sveltestrap@"git+https://github.com/bestguy/sveltestrap.git#pull/574/head" from the root project

Looks like this PR needs a package.json update to change the svelte dependency to 4.x?

LoopControl · 2023-10-11T23:12:01Z

Still getting error after the 3 commits above. When I --force the install it works with this error:

npm WARN using --force Recommended protections disabled.
npm WARN ERESOLVE overriding peer dependency
npm WARN While resolving: sveltestrap@5.11.2
npm WARN Found: svelte@4.2.1
npm WARN node_modules/svelte
npm WARN   peer svelte@"^3.54.0 || ^4.0.0-next.0" from @sveltejs/kit@1.25.0
npm WARN   node_modules/@sveltejs/kit
npm WARN     peer @sveltejs/kit@"^1.0.0" from @sveltejs/adapter-auto@2.1.0
npm WARN     node_modules/@sveltejs/adapter-auto
npm WARN     1 more (the root project)
npm WARN   9 more (@sveltejs/vite-plugin-svelte, ...)
npm WARN 
npm WARN Could not resolve dependency:
npm WARN peer svelte@"^3.53.1" from sveltestrap@5.11.2
npm WARN node_modules/sveltestrap
npm WARN   sveltestrap@"git+https://github.com/bestguy/sveltestrap.git#pull/574/head" from the root project
npm WARN 
npm WARN Conflicting peer dependency: svelte@3.59.2
npm WARN node_modules/svelte
npm WARN   peer svelte@"^3.53.1" from sveltestrap@5.11.2
npm WARN   node_modules/sveltestrap
npm WARN     sveltestrap@"git+https://github.com/bestguy/sveltestrap.git#pull/574/head" from the root project

splimter · 2023-10-12T08:37:26Z

Hi @LoopControl thank you for checking our work.
Please try with --legacy-peer-deps, it did the trick for me.

splimter · 2023-10-12T11:28:53Z

Hello @LoopControl please disregard my last message, and try again.

benmccann · 2023-10-12T13:04:28Z

@@ -0,0 +1,49 @@
+const { readdirSync, readFileSync, writeFileSync } = require("fs");


Probably don't need to check this file in

@splimter just curious if you were aware of svelte-migrate which should migrate types. Was there something missing from it or a bug or anything like that?

Hi @benmccann unfortunately no i was not aware of svelte-migrate.

@splimter do we need to consider svelte-migrate or is the necessary work already done?
Thanks!

benmccann · 2023-10-12T13:28:16Z

Thanks for this!!

milansimek · 2023-10-12T17:59:00Z

@splimter I think the peer dependency version also needs to be updated in package.json?

Right now only v3 is added:

  "peerDependencies": {
    "svelte": "^3.53.1"
  },

Most likely it needs to be updated the same as the dev dependency?

"svelte": "^3.53.1 || ^4.2.1"

milansimek · 2023-10-12T18:12:17Z

@splimter Created pull request: https://github.com/edraj/sveltestrap/pull/1

Can confirm installing without --legacy-peer-deps works after this change

Update package.json peer deps Svelte 4.0.5

kefahi · 2023-10-13T12:33:03Z

Thank you @milansimek !

@splimter I have merged the pull request against our main PR.

Kindly let me know if there is any known issues left on this front. (e..g upgrading any other dependencies)

benmccann · 2023-10-13T12:58:52Z

  },
  "peerDependencies": {
-    "svelte": "^3.53.1"
+    "svelte": "^3.53.1 || ^4.0.5"


I don't think we should leave support for Svelte 3 here. The type definitions are now using Svelte 4 types, which I don't think would be compatible. So this would only be compatible with Svelte 3 for users who aren't doing type checking. Also, there's no reason to require Svelte 4.0.5. Using any of the prior 4.x versions would be fine

Suggested change

"svelte": "^3.53.1 || ^4.0.5"

"svelte": "^4.0.0"

@benmccann Good point regarding removing Svelte 3 support 👍

benmccann · 2023-10-13T13:02:18Z

    "sirv-cli": "^1.0.12",
    "standard-version": "^9.3.0",
-    "svelte": "3.53.1",
+    "svelte": "^3.53.1 || ^4.0.5",


We can let users install a wider range of dependencies in peerDependencies, but it probably makes sense for us to test against a more specific version

Suggested change

"svelte": "^3.53.1 || ^4.0.5",

"svelte": "^4.0.5",

benmccann · 2023-10-13T13:10:12Z

Hmm. It looks like this project doesn't run the CI against PRs. Can you update the file in the .github directory to include the following?

on:
  push:
    branches:
      - master
  pull_request:

I suspect this PR will need some changes to avoid breaking the tests, but I can't tell if they're passing or not until we run them

kefahi · 2023-10-14T05:22:31Z

Hello @benmccann
Thank you for your input.
I made a number of changes and upgrades to make it work.
The CI now runs successfully.

benmccann

Thank you! This PR looks good to me now. I'd still recommend making the change I suggested earlier to when the GitHub workflow is triggered

wilmardo · 2023-10-26T17:35:24Z

Thanks for the work on this PR!
For me the modal isn't working yet, it shows up (better then the main branch). But it has no close icon and does not close when clicking next to the Modal.

Just my first Svelte project so I have no clue how to debug this further. I just copied the example Modal from the docs :)

splimter · 2023-10-29T10:18:02Z

hi @wilmardo thanks for the feedback, please make sure that the toggle prop is on both Modal and ModalHeader, lemme know if that did solve your issue.

# Conflicts: # dist/sveltestrap.es.js # dist/sveltestrap.es.js.map # dist/sveltestrap.js # dist/sveltestrap.js.map # package-lock.json # package.json

georgeemr · 2023-11-02T06:13:34Z

Can confirm this works on my current project.

SoyaNyan · 2023-11-11T14:17:03Z

I've tested this PR from our Sveltekit (Svelte4) project and there is no issue related to both modal and offcanvas.
Have to test more for completely migrate, but still looks good to me.

vdp641 · 2023-11-21T16:40:36Z

Hi, how long this PR will be merged? The modal is currently not working. I cannot use sveltestrap with Svelte 4 on cloud CI/CD.
I only fixed it in local env but in the cloud it always gets the not-working version.

splimter · 2023-11-21T16:54:54Z

Hello @vdp641 , what is the issue that you are facing with the modal.

vdp641 · 2023-11-21T17:01:04Z

@splimter hi, in the current version of sveltestrap, with Svelte 4, that does not fix the issue with global transition in Modal.svelte. It leads to Modal not opened.
This PR will fix this issue, but it has not been merged yet.

splimterXstartappz added 16 commits October 8, 2023 11:26

Fix for svelte 4 + debugs

25a541e

debugs

1545a57

debugs

da175be

debugs

5c1e067

debugs

1d2cbbd

debugs

25da92e

debugs

b15a510

debugs

1783fea

debugs

ed1dde7

debugs

35a6414

fix package version

f3ba65a

fix package version

92009d6

cleanup

50016d1

cleanup

9c699df

Fixing modal for svelte4

d44ce28

fixing .d.ts files

5150a16

kefahi mentioned this pull request Oct 9, 2023

Svelte 4 support #557

Open

splimter added 3 commits October 11, 2023 11:50

updating svelte ver in package.json

595d338

updating svelte ver in package.json

9d53e56

updating svelte ver in package.json

47c4782

updating svelte ver in package.json

28d0583

benmccann reviewed Oct 12, 2023

View reviewed changes

removing svelte4fix.js

d0433c4

Update package.json peer deps Svelte 4.0.5

2b03a4b

Merge pull request #1 from milansimek/package-json-fix

4407b87

Update package.json peer deps Svelte 4.0.5

benmccann reviewed Oct 13, 2023

View reviewed changes

Kefah Issa and others added 3 commits October 14, 2023 07:45

Fix npm test errors and upgrade packages

e4edfdd

Update linter and dependency to svelte 4.2

9c22f50

Update github runner to use node 18 and 20

c17fa04

Resolved depcreation warnings

31fad45

benmccann approved these changes Oct 14, 2023

View reviewed changes

benmccann mentioned this pull request Oct 31, 2023

Svelte4 and Storybook7 fixes #575

Open

splimter added 3 commits November 1, 2023 11:29

Merge remote-tracking branch 'origin/master' into fix-svelte4

30bc3e3

# Conflicts: # dist/sveltestrap.es.js # dist/sveltestrap.es.js.map # dist/sveltestrap.js # dist/sveltestrap.js.map # package-lock.json # package.json

fixing merge conflicts

644ac90

fix browserslist

657b846

Merged from master and resolved conflicts

19bfe3a

dysfunc mentioned this pull request Dec 15, 2023

Updates tracker sveltestrap/sveltestrap#1

Closed

59 tasks

		@@ -0,0 +1,49 @@
		const { readdirSync, readFileSync, writeFileSync } = require("fs");

Conversation

splimter commented Oct 9, 2023

Uh oh!

LoopControl commented Oct 11, 2023

Uh oh!

LoopControl commented Oct 11, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

splimter commented Oct 12, 2023

Uh oh!

splimter commented Oct 12, 2023

Uh oh!

benmccann Oct 12, 2023

Choose a reason for hiding this comment

Uh oh!

benmccann Oct 12, 2023

Choose a reason for hiding this comment

Uh oh!

splimter Oct 12, 2023

Choose a reason for hiding this comment

Uh oh!

kefahi Oct 13, 2023

Choose a reason for hiding this comment

Uh oh!

benmccann commented Oct 12, 2023

Uh oh!

milansimek commented Oct 12, 2023

Uh oh!

milansimek commented Oct 12, 2023

Uh oh!

kefahi commented Oct 13, 2023

Uh oh!

benmccann Oct 13, 2023

Choose a reason for hiding this comment

Uh oh!

milansimek Oct 13, 2023

Choose a reason for hiding this comment

Uh oh!

benmccann Oct 13, 2023

Choose a reason for hiding this comment

Uh oh!

benmccann commented Oct 13, 2023

Uh oh!

kefahi commented Oct 14, 2023

Uh oh!

benmccann left a comment

Choose a reason for hiding this comment

Uh oh!

wilmardo commented Oct 26, 2023

Uh oh!

splimter commented Oct 29, 2023

Uh oh!

georgeemr commented Nov 2, 2023

Uh oh!

SoyaNyan commented Nov 11, 2023

Uh oh!

vdp641 commented Nov 21, 2023

Uh oh!

splimter commented Nov 21, 2023

Uh oh!

vdp641 commented Nov 21, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants

LoopControl commented Oct 11, 2023 •

edited

Loading