Fix frozen string literal for ruby 3.4 #719
Conversation
chaadow
force-pushed
the
patch-1
branch
4 times, most recently
from
83ba6c9 to
5797699
Compare
|
I think I need help, for some reason the vendored JSON library is not called in acceptance tests? and I know for sure it does a string mutation because my application warns ( and crashes because i've monkey patch ruby's |
chaadow
force-pushed
the
patch-1
branch
4 times, most recently
from
e322517 to
0a75db3
Compare
|
@chaadow I think the way to fix this would be to run okjson's tests with frozen string literals enabled, then re-embed it in to Spring. It looks like the upstream repo has been archived though, so I'm not sure who is in charge of this code anymore. |
|
FWIW I ran the OKJson tests using your patch, and that fixed all of the errors there so I think this change is fine. Can you also add the "frozen string literal" directive to the top of |
chaadow
force-pushed
the
patch-1
branch
3 times, most recently
from
02a8a4b to
daf36d7
Compare
|
@tenderlove Done! thanks for the review. I do think this vendored JSON is outdated and can be switched with a more recent implementation. The CI is failing, but I can't figure out why in the errors displayed, if you have any tips please let me know. I'll keep trying to investigate when I got more time. |
|
@tenderlove Based on this ( #713) the PR was merged even though the CI was failing. and the CI errors are similar to here. So maybe, if you don't mind, we can merge this PR and repair CI in another PR? |
chaadow
force-pushed
the
patch-1
branch
2 times, most recently
from
c1d39e9 to
9680201
Compare
|
Hi @tenderlove is it possible to get a last review of this PR please? 🙏🏼 |
I think the removal of the force encoding in rails#719 was a mistake. For example, my shell prompt includes multi-byte UTF-8 characters, and when `Spring::Client::Run#run_command` collects the environment variables using `ENV.to_hash`, the resulting JSON string becomes ASCII-8BIT. This causes issues when the `Spring::Application#serve` tries to load the JSON string and passes it to `JSON.load`. Since `OkJson` expects UTF-8 encoded strings, it raises an error when it encounters the ASCII-8BIT string. I've added a test case that replicates this issue to ensure that the encoding is handled correctly. The test case creates a JSON string with multi-byte UTF-8 characters as an ASCII-8BIT string and verifies that it can be loaded without raising an error.
I think the removal of the force encoding in rails#719 was a mistake. For example, my shell prompt includes multi-byte UTF-8 characters, and when `Spring::Client::Run#run_command` collects the environment variables using `ENV.to_hash`, the resulting JSON string becomes ASCII-8BIT. This causes issues when the `Spring::Application#serve` tries to load the JSON string and passes it to `JSON.load`. Since `OkJson` expects UTF-8 encoded strings, it raises an error when it encounters the ASCII-8BIT string. I've added a test case that replicates this issue to ensure that the encoding is handled correctly. The test case creates a JSON string with multi-byte UTF-8 characters as an ASCII-8BIT string and verifies that it can be loaded without raising an error.
After running
RUBYOPT='--enable-frozen-string-literal --debug-frozen-string-literal' bin/rails c( Following this guide )I had this issue:
I went ahead and updated github actions as well, inspired by this svenfuchs/rails-i18n#1120