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

Add config.webpacker_precompile #102

Merged
merged 8 commits into from
Apr 23, 2022
Merged

Add config.webpacker_precompile #102

merged 8 commits into from
Apr 23, 2022

Conversation

Judahmeek
Copy link
Contributor

Enables the use of a webpacker.yml config value as a complement to ENV["WEBPACKER_PRECOMPILE"]

@Judahmeek Judahmeek force-pushed the judahmeek/precompile branch from 6380c56 to 3e08b80 Compare April 15, 2022 01:00
@Judahmeek Judahmeek changed the title Add gatekeeper method Add config.dont_enhance_asset_precompile Apr 15, 2022
@Judahmeek Judahmeek requested review from justin808 and tomdracz April 15, 2022 01:01
@@ -11,7 +11,7 @@ namespace :webpacker do
end
end

skip_webpacker_clean = %w(no false n f).include?(ENV["WEBPACKER_PRECOMPILE"])
skip_webpacker_clean = %w(no false n f).include?(ENV["WEBPACKER_PRECOMPILE"]) || Webpacker.config.dont_enhance_asset_precompile
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's dry up this logic

lib/webpacker/configuration.rb Outdated Show resolved Hide resolved
@Judahmeek Judahmeek requested a review from justin808 April 15, 2022 01:22
@Judahmeek Judahmeek force-pushed the judahmeek/precompile branch from 500ae0f to 622d189 Compare April 15, 2022 01:35
@Judahmeek Judahmeek force-pushed the judahmeek/precompile branch from 8d46cce to 2434a61 Compare April 15, 2022 01:41
@Judahmeek Judahmeek changed the title Add config.dont_enhance_asset_precompile Add config.webpacker_precompile Apr 15, 2022
@Judahmeek Judahmeek requested a review from justin808 April 15, 2022 02:14
ENV["WEBPACKER_PRECOMPILE"] = "yes"

assert Webpacker.config.webpacker_precompile?
end
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs to test the config file setting, if possible.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be good, might be as simple as setting different value for one of the envs in the test app for now

Copy link
Collaborator

@tomdracz tomdracz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One note for me and +1 on bits noted by @justin808

lib/install/config/webpacker.yml Show resolved Hide resolved
@justin808
Copy link
Member

@Judahmeek any ETA for getting this completed?

@Judahmeek Judahmeek force-pushed the judahmeek/precompile branch from 2bb1404 to ac0f634 Compare April 20, 2022 18:14
Copy link
Collaborator

@tomdracz tomdracz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Judahmeek Judahmeek requested a review from justin808 April 20, 2022 22:50
@Judahmeek Judahmeek marked this pull request as ready for review April 20, 2022 22:50
@@ -7,6 +7,8 @@ default: &default
public_output_path: packs
cache_path: tmp/webpacker
webpack_compile_output: true
# See https://github.com/shakacode/shakapacker#development
webpacker_precompile: true
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tomdracz tomdracz merged commit f4995d1 into master Apr 23, 2022
@Judahmeek Judahmeek deleted the judahmeek/precompile branch April 25, 2022 19:19
Eric-Guo added a commit to thape-cn/web that referenced this pull request Jun 1, 2022
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.

3 participants