Skip to content

Conversation

@StrongFennecs
Copy link
Contributor

Elixir v1.19 deprecates struct update syntax:

elixir-lang/elixir#13974

Just using map update syntax doesn't change the type of the struct, (thats still stored in the data of the map effectively), and this behaviour was always allowed for structs. It's just that elixir has decided to go forward with one syntax and build type checking features around it.

This pr removes a bunch of warnings that occur when compiling slipstream with v1.19 but still works in v1.17.

) do
{ref, state} = State.next_ref(state)
state = %State{state | joins: Map.put(state.joins, cmd.topic, ref)}
%State{} = state = %{state | joins: Map.put(state.joins, cmd.topic, ref)}
Copy link
Collaborator

Choose a reason for hiding this comment

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

For these clauses of handle_message/1, rather than matching %State{} as the return value, shouldn't we match in the function parameters?

defp handle_message(%{message: command(%Commands.JoinTopic{} = cmd), state: %State{} = state}) do
  # ..
end

I haven't been following the type system stuff lately but I would assume that this would give the type system a better hint about the original state binding, and it would know that state.joins exists on this line for example. And that state.config exists in clauses below, like the one for %Events.ChannelConnected{}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah pattern matching in the head is probably a better thing to do,

Copy link
Collaborator

@the-mikedavis the-mikedavis left a comment

Choose a reason for hiding this comment

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

Thanks!

@the-mikedavis the-mikedavis merged commit b4c16cd into CuatroElixir:main Dec 5, 2025
3 checks passed
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.

2 participants