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

Change to javascript_pack_tag automatically also include stylesheet packs if any css was emitted #2343

Closed
TylerRick opened this issue Oct 30, 2019 · 1 comment

Comments

@TylerRick
Copy link
Contributor

TylerRick commented Oct 30, 2019

Currently you have to remember to manually add a stylesheet_pack_tag for any javascript_pack_tag that has required styles. (1, 2, docs)

But I'm curious... Is there any reason why webpacker couldn't just automatically handle adding stylesheet packs for any javascript_pack_tag that is referenced on a page? (kind of like was proposed in #2202) Seems like that would be a better developer experience.

Having to manually manage whether to add a corresponding stylesheet_pack_tag for each and every javascript_pack_tag is error-prone, requires continued maintenance (since it could change over time: styles may later be added to a React component that previously needed none, for example), and can result in either of these mistakes:

  1. Forgetting to add a stylesheet_pack_tag for a JS pack that has associated styles.

As @DanielHeath noted in #2202:

Forgetting to do so results in the CSS being dropped in production but not in development (so it's an easy mistake to make!).

  1. Adding a stylesheet_pack_tag for a JS pack that doesn't have associated styles.

This could happen, for example, if you wanted to be consistent and have a matching stylesheet_pack_tag included for every javascript_pack_tag, just in case it has associated styles that you know about (either now or later)...

Or if you had a javascript_pack_tag with a dynamic pack name based on the controller action, etc. (I've done this in a test/demo controller before, for example).

The worst part of this mistake is that it may not manifest itself until production (#2342).


Or, if there's no interest in changing javascript_pack_tag to also include stylesheets, maybe add a separate helper javascript_and_stylesheet_pack_tags that emits both (or only js if there is no css)? (Name is too long, though—I wish it were js_pack_tag, css_pack_tag, and js_and_css_pack_tags.)

Ideally, it would actually check the manifest and only add the css if there was any, like maybe:

  def js_and_css_pack_tags(pack_name)
    javascript_pack_tag(pack_name) +
    if Webpacker.manifest.lookup("#{pack_name}.css")
      stylesheet_pack_tag(pack_name)
    end.to_s
  end

... though an even simpler option might be to just add a rescue:

    # Not every .js pack uses css and requesting stylesheet_pack_tag generates error if no css was emitted
  def js_and_css_pack_tags(pack_name)
    javascript_pack_tag(pack_name) +
    stylesheet_pack_tag(pack_name) rescue ''
  end
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

2 participants