search

usabilityhub / rails-erb-loader

Embedded Ruby (.erb) webpack loader for Ruby projects.
MIT License
103 stars 28 forks source link

improved dependencies directive description #108

Closed bertBruynooghe closed 2 years ago

bertBruynooghe commented 2 years ago

Make it more obvious that the javascript comment block is necessary to get dependency loading working. (Especially when not using .js.erb files.)

PikachuEXE commented 2 years ago

I think the original description should remain unchanged (or say there are pitfalls) and mention the pitfalls below with your example code <!-- /* rails-erb-loader-dependencies ... */ -->

bertBruynooghe commented 2 years ago

I took the liberty to change the original description slightly ('javascript comment block'), as that makes it clear that // rails-erb-loader-dependencies... doesn't work either.

As to including <!-- /* rails-erb-loader-dependencies */ -->: that only works for html files, while <%# /* rails-er-loader-dependencies %> works more universally (e.g. .json.erb). Adding more examples only adds to the confusion.

That being said, I still think leaving the comment signs out of the detection would make more sense than adapting the documentation, as the current implementation is a violation of the Principle of Least Astonishment, at the heart of ruby and rails.

PikachuEXE commented 2 years ago

Ya just think updating the README is a faster improvement Let's keep #107 open and see if further improvements can be made