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

Update structure and add webpacker #6

Merged
merged 2 commits into from
Jun 26, 2020

Conversation

rojasTob
Copy link
Contributor

I've updated the structure using the framework's command and install webpacker gem.

Copy link
Contributor

@mmena1 mmena1 left a comment

Choose a reason for hiding this comment

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

Please take a look at the requested changes. Everything else is fine 🙂

Comment on lines 3 to 13
default: &default
source_path: app/javascript
source_entry_path: packs
public_root_path: public
public_output_path: packs
cache_path: tmp/cache/webpacker
webpack_compile_output: true

# Additional paths webpack should lookup modules
# ['app/assets', 'engine/foo/app/assets']
resolved_paths: []
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you are not using webpack at all. As you can see here, webpack looks for assets in app/javascript. You have 2 options here:

  1. Move all the existing assets to app/javascript (.js go in app/javascript/packs).
  2. Or, add app/assets on the list of resolved_paths

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, I forgot it. I've added the assets path on the list.

Copy link
Contributor

Choose a reason for hiding this comment

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

We also need to modify the app/javascript/packs/application.js to include the assets which are going to be served through webpack and move all assets to app/javascript as described on the webpacker documentation. Since we are short on time and this is not a major issue, we can postpone this for a later update.
NOTE: This applies to all other rails tutorials which @rojasTob updated.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks @mrcrow85 for your review. About the last improvement, I will create an issue explaining the last step that it needs to run Webpack. I will create it for each Rails repository.

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.

None yet

2 participants