vt-elixir / ja_serializer

JSONAPI.org Serialization in Elixir.
Other
640 stars 148 forks source link

Fix compilation error when defining already inlined by compiler methods #304

Closed marpo60 closed 5 years ago

marpo60 commented 5 years ago

Fixes https://github.com/vt-elixir/ja_serializer/issues/283 Related https://github.com/vt-elixir/ja_serializer/pull/245

Benchmark before the change:

Settings:
  duration:      1.0 s

## JaSerializer.PhoenixViewBench
[17:16:16] 1/6: attributes map big data
[17:16:21] 2/6: attributes map small data
[17:16:23] 3/6: render index 25 items map big data
[17:16:25] 4/6: render index 25 items small data
[17:16:29] 5/6: render index one item map big data
[17:16:32] 6/6: render index one item small data

Finished in 17.87 seconds

## JaSerializer.PhoenixViewBench
attributes map small data             10000000   0.12 µs/op
attributes map big data               10000000   0.46 µs/op
render index one item small data         20000   84.84 µs/op
render index one item map big data       20000   93.65 µs/op
render index 25 items small data         10000   292.75 µs/op
render index 25 items map big data        2000   820.56 µs/op

Benchmark after the change

Settings:
  duration:      1.0 s

## JaSerializer.PhoenixViewBench
[17:15:36] 1/6: attributes map big data
[17:15:42] 2/6: attributes map small data
[17:15:43] 3/6: render index 25 items map big data
[17:15:46] 4/6: render index 25 items small data
[17:15:48] 5/6: render index one item map big data
[17:15:50] 6/6: render index one item small data

Finished in 17.07 seconds

## JaSerializer.PhoenixViewBench
attributes map small data             10000000   0.13 µs/op
attributes map big data               10000000   0.53 µs/op
render index one item small data         20000   83.12 µs/op
render index one item map big data       20000   92.74 µs/op
render index 25 items small data          5000   288.62 µs/op
render index 25 items map big data        2000   785.51 µs/op

Best!

beerlington commented 5 years ago

Awesome, thanks!

beerlington commented 5 years ago

@marpo60 I just tried running this on my production app and am getting errors related to overriding attributes in the view like this:

defmodule MyView do
  use JaSerializer.PhoenixView

  attributes([:my_attr])

  def my_attr(%{my_attr: my_attr}), do: my_attr
end

produces...

function MyView.my_attr/1 is undefined or private

I haven't had a chance to dig in too much, but maybe there isn't test coverage around this or my app is doing something we're not testing. I might need to revert this if I can't figure out what's going on.

marpo60 commented 5 years ago

@beerlington I just tried both cases on a local branch and they are working for me. Im not able to reproduce the bug 😕

https://github.com/marpo60/ja_serializer/commit/2af4ee9a968a1e48ce4ba629d71e31ea1bb5318a

beerlington commented 5 years ago

@marpo60 I dug in a little more and think I created a test that reproduces it. I want to spend a little more time with it this evening and see if it's something we're doing in our production app or not.