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

Attribute view box is missing since solid-js 1.6 #15

Closed
pmwmedia opened this issue Oct 22, 2022 · 17 comments
Closed

Attribute view box is missing since solid-js 1.6 #15

pmwmedia opened this issue Oct 22, 2022 · 17 comments

Comments

@pmwmedia
Copy link

Bug:

When using solid-js 1.6.0, the attribute viewBox is missing for all SVGs.

Workaround:

Downgrading solid-js to version 1.5.9 or earlier.

@pmwmedia
Copy link
Author

Is this the cause for the bug: solidjs/solid#1296?

@oscartbeaumont
Copy link

I also had this issue but even after downgrading SolidJS to 1.5.9 many icons were still displaying weirdly. I ended up resetting back to an old lock file of mine in had in Git.
The issue @pmwmedia linked said it should be fixed if you remove you lock file and reinstall but that didn't seem to be the case on my machine.

@outbackStack
Copy link

outbackStack commented Oct 23, 2022

downgrading to 1.5.4 works for me...

@x64Bits
Copy link
Owner

x64Bits commented Oct 25, 2022

Hi everyone, if there seems to be a problem when destructuring the props, do you think it is convenient to establish a specific version of solid-js as a dependency for solid-icons?

Because right now { "solid-js" : "*" } is set for convenience and directly use the same user version but if this type of problem can happen I think it's better to have a fixed version.

@outbackStack
Copy link

hi @x64Bits,

Thanks for creating this for us :-)

Feather icons has specific version but since solid changes every 3 months it's best to leave it as it is. We can just report the problem if people run into.

Otherwise, you will need to update this constantly....

@Schlumie
Copy link

Will this be fixed? Downgrade cant be the solution?

@icode
Copy link

icode commented Oct 27, 2022

downgrading to 1.5.7 works for me...

@x64Bits
Copy link
Owner

x64Bits commented Oct 27, 2022

I'm sending version 1.0.3 which adds solid-js <=1.5.7 as a dependency to avoid the problem, I did not want to have a direct dependency on solid since it was implicit but if is not added the library it breaks, If anyone has a better idea I'll leave this thread open for suggestions.

@outbackStack
Copy link

@x64Bits with v1.0.3 you are forcing everyone to wait for this package to get updated before they can update solid-js. Breaking changes like this rarely occur for icons so I would suggest to leave it as it is and we just report the bug.

@x64Bits
Copy link
Owner

x64Bits commented Oct 28, 2022

@outbackStack I understand, because I don't like the idea of forcing a version either, but it tells me that people will choose to downgrade solid-js in order to use this library? Doesn't it sound more logical to update this lib to make it work?

@outbackStack
Copy link

outbackStack commented Oct 28, 2022

@x64Bits I've seen packages where people don't maintain it anymore and we are all stuck even though the latest version still works. It's up to you on how much time you have to maintain this. I'm just thinking it will be easier for you without dependency requirement....

Anyhow, if it does not work with the latest version of solid-js, issues will be created by users of this package.

Thanks again for creating this.

@icode
Copy link

icode commented Oct 28, 2022

@outbackStack I understand, because I don't like the idea of forcing a version either, but it tells me that people will choose to downgrade solid-js in order to use this library? Doesn't it sound more logical to update this lib to make it work?

We are waiting for solidjs to fix this problem!

@outbackStack
Copy link

outbackStack commented Oct 28, 2022

@icode @x64Bits
I'm sorry I didn't realised the issue was with solid-js new destructing props that can only be fixed by them....

@pmwmedia
Copy link
Author

pmwmedia commented Oct 30, 2022

According to @ryansolid in solidjs/solid#1296 (comment). All spread issues should be fixed in 1.6.1. Unfortunately, the view box is still missing for all SVGs when using solid-icons 1.0.3 together with solid-js 1.6.1.

Ok 1.6.1 fixed all the open issues we have reported with reproductions against spreads. Since the original issue wasn't actually an issue I'm going to close this one. And if you find issues against the new version please report them.

@icode
Copy link

icode commented Oct 30, 2022

According to @ryansolid in solidjs/solid#1296 (comment). All spread issues should be fixed in 1.6.1. Unfortunately, the view box is still missing for all SVGs when using solid-icons 1.0.3 together with solid-js 1.6.1.

Ok 1.6.1 fixed all the open issues we have reported with reproductions against spreads. Since the original issue wasn't actually an issue I'm going to close this one. And if you find issues against the new version please report them.

Yes, I use solid-js 1.6.1, it not fixed this problem.

@x64Bits
Copy link
Owner

x64Bits commented Oct 30, 2022

Everything seems to indicate that the changes I made in 1.0.3 had no effect in general, mergeProps seems to help fix this,
i'm submitting another version for this in the PR#17 feat: add unit tests.

@pmwmedia
Copy link
Author

pmwmedia commented Nov 1, 2022

I can confirm that version 1.0.4 fixes this issue.

@x64Bits Thank you very much for the bug fix!

@pmwmedia pmwmedia closed this as completed Nov 1, 2022
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

When branches are created from issues, their pull requests are automatically linked.

6 participants