codeforamerica / honeycrisp-gem

A Rails gem with base styles and Javascript for Code for America products
http://honeycrisp.herokuapp.com
MIT License
24 stars 8 forks source link

Pull JS in main sprockets file into its own file (honeycrisp.js) #241

Closed bytheway875 closed 3 years ago

bytheway875 commented 3 years ago

On GetYourRefund, we're currently using both webpacker AND sprockets for JS. I'd like to make the move to building all of our JS in webpacker so that we don't run into conflicts on jquery versions, and to reinforce writing more modularized, testable javascript on the team.

When trying to import the cfa_styleguide_main js file directly and webpacking it up, we run into issues because of the require statements at the top of this sprockets manifest. By pulling the contents of the javascript file out into its own file, we maintain current functionality for projects using sprockets but also allow for it to be imported into a webpacker set up.

(I'd initially tried to modularize the honeycrisp.js file so that I could import it into webpack as a module and keep it out of the global namespace, but all of the sprockets modularization gems seem to have been abandoned ~5 years ago when Rails made the decision to move towards using webpack for asset modularization.)

hartsick commented 3 years ago

I don't think I have enough context about Javascript loading to be a useful reviewer! I'd happily do some research about this if it's helpful, but don't want to be a blocker.

bytheway875 commented 3 years ago

@bensheldon Re: Modularization + encapsulation. I guess there are ways to encapsulate this better without it being a module, but when I tried to modularize it even with an old style module.exports, sprockets needed extra configuration on how to deal with it, and none of those sprockets gems seemed to have enough support to rely on. So once I was fighting with that for a while, I was just like okay whatever let me just move the file so I can import it. But it's still adding to the global namespace, and it's still "init"-ing as soon as the code is imported. :( I probably need to google "patterns for encapsulating javascript with sprockets" 😂

bensheldon commented 3 years ago

I think this PR is good to go.

I'd be happy to pair or just chat about the JS. My thinking is to break it into two files. One file (or more) that has the implementation code and is not self-initializing and that could be used with Webpacker. And another file for backwards compatibility/sprockets that does the self-initializing.

bytheway875 commented 3 years ago

@bensheldon Ah, yes. I tried something like that here and then was struggling with it! https://github.com/codeforamerica/honeycrisp-gem/pull/241/commits/e69abd7bb63699082df8abeaba992a1c50b89a35

Would love to revisit but will merge as-is for now.