Skip to content

Conversation

TBK145
Copy link
Contributor

@TBK145 TBK145 commented Apr 24, 2025

OTP 28 changes the implementation of the regex module, and as a result you can no longer use regexes as module attributes.

By changing the regex to a pattern as a string it can still be used in the error message, and just needs to be compiled before usage.

@benwilson512
Copy link
Contributor

Hey @TBK145 is there some Elixir issue tracking this? Recompiling the regex on every function call is going to be much worse for performance.

@valid_name_regex "^[_A-Za-z][_0-9A-Za-z]*$"

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
@valid_name_regex "^[_A-Za-z][_0-9A-Za-z]*$"

Comment on lines 33 to 35
@valid_name_regex
|> Regex.compile!()
|> Regex.match?(name)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
@valid_name_regex
|> Regex.compile!()
|> Regex.match?(name)
Regex.match?(~r/^[_A-Za-z][_0-9A-Za-z]*$/, name)

@josevalim
Copy link
Contributor

This is a change on Erlang/OTP 28, we can no longer precompile them in the module body, but we only need to move it inside. Elixir v1.19 will emit a warning telling you to do exactly that.

@TBK145
Copy link
Contributor Author

TBK145 commented Apr 29, 2025

Hi @josevalim , at first I wanted to solve it in the way you propose, but the regex is also referenced here, so I did it in this way to only have to maintain the regex in one place.
But I understand it's better to maintain the regex twice in favour of performance, thanks @benwilson512 I did not think about that.

@josevalim
Copy link
Contributor

In this case it is best to move the regex to a private function and reuse it. :)

@TBK145
Copy link
Contributor Author

TBK145 commented Apr 30, 2025

That was actually the first thing I tried, but since it's a module attribute, the function is not defined yet before calling.
I could move the whole @description block into the function, but I don't know if that has any negative side-effects.

@josevalim
Copy link
Contributor

What I mean is this:

defp valid_regex_name, do: ~r/foobar/

And use it as valid_regex_name().

@TBK145
Copy link
Contributor Author

TBK145 commented Apr 30, 2025

I understand, but what I mean is that the regex is also used in module attribute @description, which cannot call a function from that same module.

@josevalim
Copy link
Contributor

You can also move it to a function. I assume there isn't much benefit for those being attributes anyway, besides user convenience.

OTP 28 changes the implementation of the regex module, and as a
result you can no longer use regexes as module attributes.
@benwilson512
Copy link
Contributor

Thank you @josevalim for the assist!

@benwilson512 benwilson512 merged commit 2c0a347 into absinthe-graphql:main Apr 30, 2025
0 of 10 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.

3 participants