Skip to content

updates required for go 1.18 #2

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

Closed
wants to merge 1 commit into from

Conversation

mlctrez
Copy link
Contributor

@mlctrez mlctrez commented Apr 27, 2022

This PR contains the changes required to allow go-indexeddb to work with go 1.18

golang/go#44006 removed js.Wrapper
KeyRange type is now passed as keyRange.Value in Calls

js.ValueOf now appears to be a bit more strict on types
Various types like CursorDirection are now passed using .String() to Calls

@chux0519
Copy link

@mlctrez Saved my day, I also encountered the same problem and guide me to the same issue. thanks

Copy link
Contributor

@JohnStarich JohnStarich left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @mlctrez for opening! ❤️

This is a great start and a much needed fix. Since there are some breaking changes, I thought it'd be helpful to make a few additional breaking changes to better align with Go 1.18, so I've included your commits in my new PR.

@@ -1,3 +1,3 @@
module github.com/hack-pad/go-indexeddb

go 1.16
go 1.18
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like to revert this piece, since this requires all modules that import go-indexeddb to use Go 1.18 or higher. I'll add go test runs to ensure it's forward-compatible with Go 1.18 too.

Comment on lines 79 to +80
// JSValue implements js.Wrapper
func (k *KeyRange) JSValue() js.Value {
return k.jsKeyRange
}
// removed see : https://github.com/golang/go/issues/44006
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's try to keep the struct field private (i.e. not embedded) and switch to using k.jsKeyRange.

Since we're in the same package and can't use the old js.Wrapper, we don't need to export it anymore. 👍 We can remove this old doc comment too.

This is technically a breaking change, so I'll mark the release of this change with a v0.x minor version bump.

@JohnStarich
Copy link
Contributor

Merged via #3

@JohnStarich
Copy link
Contributor

Released in v0.2.0 🎉
If you hit any issues, let me know!

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

Successfully merging this pull request may close these issues.

3 participants