Add troubleshooting section around static paths output when using webpack context #547
Conversation
WalkthroughThe pull request adds a new section to the troubleshooting documentation focusing on static file dependencies in Webpack configuration. The documentation explains a potential issue with asset path handling when the Webpack Changes
Possibly related PRs
Poem
Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
docs/troubleshooting.md (4)
232-238: Minor grammatical improvements neededConsider these grammatical improvements:
- Add a comma after "however" in line 238
- Add a comma after "By default" in line 238
- Simplify "In order to" to "To" in line 236 for conciseness
-This usually works out of the box. There's a potential problem however, if you use the +This usually works out of the box. There's a potential problem, however, if you use the🧰 Tools
🪛 LanguageTool
[style] ~235-~235: Consider a shorter alternative to avoid wordiness.
Context: ...tic` folder in the public output path. In order to generate the storage path, we rely on t...(IN_ORDER_TO_PREMIUM)
[formatting] ~238-~238: Consider inserting a comma before ‘however’.
Context: ...rks out of the box. There's a potential problem however, if you use the [context setting](https:...(HOWEVER_MISSING_COMMA)
[uncategorized] ~238-~238: Did you mean: “By default,”?
Context: ...ntext/#context) in your webpack config. By default this is set to current Node working dir...(BY_DEFAULT_COMMA)
241-245: Add language identifier to code blockAdd
jsorjavascriptto the code fence for proper syntax highlighting:-``` +```js { context: path.resolve(__dirname, '../../app/javascript') }<details> <summary>🧰 Tools</summary> <details> <summary>🪛 Markdownlint (0.37.0)</summary> 241-241: null Fenced code blocks should have a language specified (MD040, fenced-code-language) </details> </details> --- `249-252`: **Standardize bullet point style** For consistency with Markdown style guidelines, use asterisks instead of dashes for bullet points: ```diff -a static asset from `node_modules` folder could end up being referenced with path of `../../node_modules/some_module/static_file.jpg` rather than simply `node_modules/some_module/static_file.jpg`. -a static asset in one of the `additional_paths`, example `app/assets/images/image.jpg`, would end up being referenced with path of `../assets/images/image.jpg`. +* a static asset from `node_modules` folder could end up being referenced with path of `../../node_modules/some_module/static_file.jpg` rather than simply `node_modules/some_module/static_file.jpg`. +* a static asset in one of the `additional_paths`, example `app/assets/images/image.jpg`, would end up being referenced with path of `../assets/images/image.jpg`.🧰 Tools
🪛 Markdownlint (0.37.0)
250-250: Expected: asterisk; Actual: dash
Unordered list style(MD004, ul-style)
251-251: Expected: asterisk; Actual: dash
Unordered list style(MD004, ul-style)
255-258: Standardize bullet points and approve contentThe solutions provided are clear and helpful. For consistency, update the bullet points:
-not using overridden `context` in your webpack config, if there's no good reason for it. -using custom Webpack config to modify the static file rule, following a similar process as outlined in the [Webpack Configuration](https://github.com/shakacode/shakapacker/blob/main/README.md#webpack-configuration) section of the readme. +* not using overridden `context` in your webpack config, if there's no good reason for it. +* using custom Webpack config to modify the static file rule, following a similar process as outlined in the [Webpack Configuration](https://github.com/shakacode/shakapacker/blob/main/README.md#webpack-configuration) section of the readme.🧰 Tools
🪛 Markdownlint (0.37.0)
256-256: Expected: asterisk; Actual: dash
Unordered list style(MD004, ul-style)
257-257: Expected: asterisk; Actual: dash
Unordered list style(MD004, ul-style)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docs/troubleshooting.md(1 hunks)
🧰 Additional context used
🪛 LanguageTool
docs/troubleshooting.md
[style] ~235-~235: Consider a shorter alternative to avoid wordiness.
Context: ...tic` folder in the public output path. In order to generate the storage path, we rely on t...
(IN_ORDER_TO_PREMIUM)
[formatting] ~238-~238: Consider inserting a comma before ‘however’.
Context: ...rks out of the box. There's a potential problem however, if you use the [context setting](https:...
(HOWEVER_MISSING_COMMA)
[uncategorized] ~238-~238: Did you mean: “By default,”?
Context: ...ntext/#context) in your webpack config. By default this is set to current Node working dir...
(BY_DEFAULT_COMMA)
[uncategorized] ~240-~240: Possible missing comma found.
Context: .../project root. If you were to override it like: ``` { context: path.resolve(__d...
(AI_HYDRA_LEO_MISSING_COMMA)
🪛 Markdownlint (0.37.0)
docs/troubleshooting.md
250-250: Expected: asterisk; Actual: dash
Unordered list style
(MD004, ul-style)
251-251: Expected: asterisk; Actual: dash
Unordered list style
(MD004, ul-style)
256-256: Expected: asterisk; Actual: dash
Unordered list style
(MD004, ul-style)
257-257: Expected: asterisk; Actual: dash
Unordered list style
(MD004, ul-style)
241-241: null
Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ Context from checks skipped due to timeout of 90000ms (13)
- GitHub Check: Generator specs (ubuntu-latest, 3.1, gemfiles/Gemfile-rails.7.1.x)
- GitHub Check: Generator specs (ubuntu-latest, 3.1, gemfiles/Gemfile-rails.7.0.x)
- GitHub Check: Generator specs (ubuntu-latest, 3.1, gemfiles/Gemfile-rails.6.1.x)
- GitHub Check: Generator specs (ubuntu-latest, 3.0, gemfiles/Gemfile-rails.7.1.x)
- GitHub Check: Generator specs (ubuntu-latest, 3.0, gemfiles/Gemfile-rails.7.0.x)
- GitHub Check: Generator specs (ubuntu-latest, 3.0, gemfiles/Gemfile-rails.6.1.x)
- GitHub Check: Generator specs (ubuntu-latest, 3.0, gemfiles/Gemfile-rails.6.0.x)
- GitHub Check: Testing (ubuntu-latest, 2.7, gemfiles/Gemfile-rails.7.0.x)
- GitHub Check: Generator specs (ubuntu-latest, 2.7, gemfiles/Gemfile-rails.7.1.x)
- GitHub Check: Generator specs (ubuntu-latest, 2.7, gemfiles/Gemfile-rails.7.0.x)
- GitHub Check: Generator specs (ubuntu-latest, 2.7, gemfiles/Gemfile-rails.6.1.x)
- GitHub Check: Generator specs (ubuntu-latest, 2.7, gemfiles/Gemfile-rails.6.0.x)
- GitHub Check: test
🔇 Additional comments (1)
docs/troubleshooting.md (1)
232-259: Documentation changes look great! 👍The new troubleshooting section effectively addresses the issue with static file paths when using Webpack context configuration. The explanation is clear, examples are helpful, and solutions are practical. The technical content is accurate and well-referenced.
🧰 Tools
🪛 LanguageTool
[style] ~235-~235: Consider a shorter alternative to avoid wordiness.
Context: ...tic` folder in the public output path. In order to generate the storage path, we rely on t...(IN_ORDER_TO_PREMIUM)
[formatting] ~238-~238: Consider inserting a comma before ‘however’.
Context: ...rks out of the box. There's a potential problem however, if you use the [context setting](https:...(HOWEVER_MISSING_COMMA)
[uncategorized] ~238-~238: Did you mean: “By default,”?
Context: ...ntext/#context) in your webpack config. By default this is set to current Node working dir...(BY_DEFAULT_COMMA)
[uncategorized] ~240-~240: Possible missing comma found.
Context: .../project root. If you were to override it like: ``` { context: path.resolve(__d...(AI_HYDRA_LEO_MISSING_COMMA)
🪛 Markdownlint (0.37.0)
250-250: Expected: asterisk; Actual: dash
Unordered list style(MD004, ul-style)
251-251: Expected: asterisk; Actual: dash
Unordered list style(MD004, ul-style)
256-256: Expected: asterisk; Actual: dash
Unordered list style(MD004, ul-style)
257-257: Expected: asterisk; Actual: dash
Unordered list style(MD004, ul-style)
241-241: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
|
Thanks @tomdracz |
Summary
Related to issue here #538
Add troubleshooting section to the readme explaining the case where using
contextconfig in Webpack could result in incorrect output of static file paths.Pull Request checklist
Add/update test to cover these changesUpdate CHANGELOG fileSummary by CodeRabbit