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

feat(helper): add unmanifested pack tag helpers #22

Closed
wants to merge 1 commit into from

Conversation

robwise
Copy link

@robwise robwise commented Jun 25, 2017

Resolves #21


This change is Reviewable

@robwise robwise requested a review from justin808 June 25, 2017 11:10
@justin808
Copy link
Member

I'm not clear on this one. A small example would be very helpful.

This sounds a bit like hot reloading in that there's no hash and special handling.

Could we do this with more options to the javascript_pack_tag rather than new helpers?


Review status: 0 of 3 files reviewed at latest revision, 1 unresolved discussion.


lib/webpacker_lite/helper.rb, line 46 at r1 (raw file):

  #   # In production mode:
  #   <%= javascript_pack_tag 'calendar', 'data-turbolinks-track': 'reload' %> # =>
  #   <script src="/public/webpack/production/calendar.js" data-turbolinks-track="reload"></script>

I don't get how you output a non-hashed file in production mode.


Comments from Reviewable

@robwise
Copy link
Author

robwise commented Jun 26, 2017

I'm not clear on this one. A small example would be very helpful.

I wrote a detailed explanation in the issue (#21). Did you read that?

Could we do this with more options to the javascript_pack_tag rather than new helpers?

Yes, this is a style thing. We could either have a new method or keep the same method with a boolean flag. Usually, I get yelled at by the guys for doing boolean flags as that's considered a bit of a code smell, but I suppose we could do manifested: false or some such and default it to true? But then we'd have to pick it out of the rested named params which would be annoying.


Review status: 0 of 3 files reviewed at latest revision, 1 unresolved discussion.


lib/webpacker_lite/helper.rb, line 46 at r1 (raw file):

Previously, justin808 (Justin Gordon) wrote…

I don't get how you output a non-hashed file in production mode.

Well, that's easy, just don't tell webpack to do it. I think you mean why would you? You probably wouldn't since you would have major caching problems. I can remove that part of the documentation, but technically, it is possible.


Comments from Reviewable

@justin808
Copy link
Member

@robwise any comments on #20?

@robwise
Copy link
Author

robwise commented Jul 4, 2017

@justin808 What do I need to do to get this merged? Switch to a flag?

@robwise
Copy link
Author

robwise commented Jul 5, 2017

Closing because you can use the cache option for the manifest plugin to share a manifest between configs. See rails/webpacker#464 (comment)

@robwise robwise closed this Jul 5, 2017
@robwise robwise deleted the rob/unmanifested-asset-helper branch July 5, 2017 16:21
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

Successfully merging this pull request may close these issues.

2 participants