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

Converter raises a parse error when interpolation is use in attribute that are enclosed by "" #409

Closed
Malian opened this issue May 30, 2021 · 7 comments

Comments

@Malian
Copy link
Collaborator

Malian commented May 30, 2021

The converter returns an error with the following code:

<img class="h-8 w-8 rounded-full" src="{{ Routes.static_path(@socket, "/images/empty_user_avatar.jpeg") }}" alt="">

The error:

mix surface.convert_syntax failed for file: lib/my_app_web/live/pages/main_page_live.sface
** (Surface.Compiler.ParseError) nofile:48:47: expected attribute name
    (surface 0.4.0) lib/surface/compiler/tokenizer.ex:558: Surface.Compiler.Tokenizer.handle_attribute/5
    (surface 0.4.0) lib/surface/compiler/converter.ex:17: Surface.Compiler.Converter.convert/2
    (surface 0.4.0) lib/mix/tasks/surface/convert_syntax.ex:85: Mix.Tasks.Surface.ConvertSyntax.convert_file/2
    (elixir 1.12.0) lib/task/supervised.ex:90: Task.Supervised.invoke_mfa/2
    (elixir 1.12.0) lib/task/supervised.ex:35: Task.Supervised.reply/5
    (stdlib 3.15) proc_lib.erl:226: :proc_lib.init_p_do_apply/3

If I remove the "" that encloses the interpolation, it works as expected:

<img class="h-8 w-8 rounded-full" src={{ Routes.static_path(@socket, "/images/empty_user_avatar.jpeg") }} alt="">
@msaraiva
Copy link
Collaborator

@Malian this one is going to be tough. Since the new Tokenizer doesn't accept embedded expressions in strings, it will always close the string on the second " found, which is the first one inside the expression. We can simulate the problem with this simpler snippet:

<img src="{{ "/" }}">

which will have the attribute value parsed as "{{ ", leaving the tokenizer expecting either another attribute name or the end of the tag. I'm not sure if it's worth adjusting the tokenizer to handle that just for converter. We could document this edge case in the "Limitations" section and instruct the user to fix it manually.

I know it's not ideal but I'm not sure we should make the tokenizer more complex just to handle that. I don't expect seeing this very often. WDYT?

/cc @lnr0626, @miguel-s

@lnr0626
Copy link
Collaborator

lnr0626 commented May 31, 2021

I've used interpolation embedded in strings in at least a few places throughout my app, so it'd be nice to make it so the converter could handle this correctly

@Malian if you try using the lr-compat-mode branch, does that solve this issue?

@Malian
Copy link
Collaborator Author

Malian commented May 31, 2021

@lnr0626 no, I have the same issue with the lr-compat-mode 😢

In my main project, I counted 15 times this issue. To fix it I was looking for "{.

I do not know how much time it will take to update the tokenizer, but as we will remove this part of the code in future versions, we could maybe handle this. Another solution is to merge the current code into master and wait for community feedback.

@msaraiva
Copy link
Collaborator

Ok. I'll try to come up with the simplest solution possible so we can remove it later.

@msaraiva
Copy link
Collaborator

@Malian this PR #412 should do the trick. Please let me know if you still run into any other issue.

@Malian
Copy link
Collaborator Author

Malian commented May 31, 2021

@msaraiva It works! Thanks ❤️

@msaraiva
Copy link
Collaborator

Fixed in #412.

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

No branches or pull requests

3 participants