Docs/auto bundle rename and dummy reference #1758
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughUpdated the automated bundle generation guide: replaced BarComponentTwo with SpecialComponentNotToAutoLoadBundle across examples, expanded dummy-app examples (nested entries, source_path, components_subdirectory=startup, auto_load_bundle), added server bundle import and paired client/server examples, and added notes about removing generalized ComponentName.jsx. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Assessment against linked issues
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
docs/guides/file-system-based-automated-bundle-generation.md (3)
47-49: Make both references repo-relative.Consistency and durability for internal links.
Apply:
-> [Dummy initializer](https://github.com/shakacode/react_on_rails/blob/master/spec/dummy/config/initializers/react_on_rails.rb) +> [Dummy initializer](../../spec/dummy/config/initializers/react_on_rails.rb)
160-163: Hyphenation and brevity tweak for clarity.Use “file-system-based” and remove the redundant parenthetical.
Apply:
-> For example, if you wanted to utilize our file-system based entrypoint generation for `FooComponentOne` and `BarComponentOne`, but not `SpecialComponentNotToAutoLoadBundle` (formerly `BarComponentTwo`) (for whatever reason), then... +For example, if you want to use our file-system-based entrypoint generation for `FooComponentOne` and `BarComponentOne`, but not `SpecialComponentNotToAutoLoadBundle` (formerly `BarComponentTwo`), then...
34-35: Prefer repo-relative link to the initializer. Keeps docs portable across branches.Apply:
-> [Dummy initializer](https://github.com/shakacode/react_on_rails/blob/master/spec/dummy/config/initializers/react_on_rails.rb) +> [Dummy initializer](../../spec/dummy/config/initializers/react_on_rails.rb)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
docs/guides/file-system-based-automated-bundle-generation.md(7 hunks)
🧰 Additional context used
🧠 Learnings (5)
📚 Learning: 2025-04-26T21:55:55.874Z
Learnt from: alexeyr-ci2
PR: shakacode/react_on_rails#1732
File: spec/dummy/client/app-react16/startup/ReduxSharedStoreApp.client.jsx:40-44
Timestamp: 2025-04-26T21:55:55.874Z
Learning: In the react_on_rails project, files under `app-react16` directories are copied/moved to corresponding `/app` directories during the conversion process (removing the `-react16` suffix), which affects their relative import paths at runtime.
Applied to files:
docs/guides/file-system-based-automated-bundle-generation.md
📚 Learning: 2025-02-12T16:38:06.537Z
Learnt from: Romex91
PR: shakacode/react_on_rails#1697
File: package-scripts.yml:28-28
Timestamp: 2025-02-12T16:38:06.537Z
Learning: The file `node_package/lib/ReactOnRails.full.js` is autogenerated during the build process and should not be present in the repository.
Applied to files:
docs/guides/file-system-based-automated-bundle-generation.md
📚 Learning: 2024-10-08T20:53:47.076Z
Learnt from: theforestvn88
PR: shakacode/react_on_rails#1620
File: spec/dummy/client/app/startup/HelloTurboStream.jsx:3-3
Timestamp: 2024-10-08T20:53:47.076Z
Learning: The `RailsContext` import in `spec/dummy/client/app/startup/HelloTurboStream.jsx` is used later in the project, as clarified by the user theforestvn88.
Applied to files:
docs/guides/file-system-based-automated-bundle-generation.md
📚 Learning: 2025-02-13T19:09:15.991Z
Learnt from: AbanoubGhadban
PR: shakacode/react_on_rails#1644
File: node_package/src/RSCWebpackLoader.ts:0-0
Timestamp: 2025-02-13T19:09:15.991Z
Learning: In React Server Components webpack loader, using `new Function('return import("react-server-dom-webpack/node-loader")')()` is necessary as a workaround to bypass TypeScript compilation issues with direct dynamic imports.
Applied to files:
docs/guides/file-system-based-automated-bundle-generation.md
📚 Learning: 2025-07-08T05:57:29.630Z
Learnt from: AbanoubGhadban
PR: shakacode/react_on_rails#1745
File: node_package/src/RSCRequestTracker.ts:8-14
Timestamp: 2025-07-08T05:57:29.630Z
Learning: The global `generateRSCPayload` function in React on Rails Pro (RORP) is provided by the framework during rendering requests, not implemented in application code. The `declare global` statements are used to document the expected interface that RORP will inject at runtime.
Applied to files:
docs/guides/file-system-based-automated-bundle-generation.md
🪛 LanguageTool
docs/guides/file-system-based-automated-bundle-generation.md
[grammar] ~34-~34: There might be a mistake here.
Context: ...d startup instead of ror_components. > [Dummy initializer](https://github.com...
(QB_NEW_EN)
[grammar] ~76-~76: There might be a mistake here.
Context: ...see the server bundle entrypoint import. > [Dummy server-bundle.js](https://githu...
(QB_NEW_EN)
[grammar] ~164-~164: There might be a mistake here.
Context: ...ipt_pack_tagandstylesheet_pack_tag`. 2. Remove generated entrypoints from parame...
(QB_NEW_EN)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: dummy-app-integration-tests (newest)
- GitHub Check: dummy-app-integration-tests (oldest)
- GitHub Check: rspec-package-tests (newest)
- GitHub Check: rspec-package-tests (oldest)
🔇 Additional comments (6)
docs/guides/file-system-based-automated-bundle-generation.md (6)
170-172: LGTM: layout shows manual inclusion for the excluded bundle.Good illustration of excluding a component from auto-loading while keeping it explicitly in the layout.
179-179: LGTM: packs entrypoint for the non-auto-loaded component.Matches the narrative and expected structure.
191-191: LGTM: placing the component outside the components_subdirectory.Accurately conveys why it won’t be auto-registered.
222-225: LGTM: clear guidance on .server/.client split with concrete dummy links.This section reads well and aligns with typical setups.
20-21: Use a repo-relative link for the dummy shakapacker.ymlReplace the hardcoded GitHub URL with a relative path to prevent link rot:
-> [Dummy shakapacker.yml](https://github.com/shakacode/react_on_rails/blob/master/spec/dummy/config/shakapacker.yml) +> [Dummy shakapacker.yml](../../spec/dummy/config/shakapacker.yml)
76-77: Use a repo-relative link for the dummy server-bundle import-> [Dummy server-bundle.js](https://github.com/shakacode/react_on_rails/blob/master/spec/dummy/client/app/packs/server-bundle.js) +> [Dummy server-bundle.js](../../spec/dummy/client/app/packs/server-bundle.js)
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
docs/guides/file-system-based-automated-bundle-generation.md (1)
196-196: Correct helper: append_javascript_pack_tag.This fixes the earlier
_tagomission flagged in prior review. Looks good.
🧹 Nitpick comments (5)
docs/guides/file-system-based-automated-bundle-generation.md (5)
20-22: Inline the dummy config snippet for clarity.After the link, add a short YAML excerpt showing
source_path: client/appwithnested_entries: trueso readers don’t have to click through.
34-36: Tighten wording and avoid potential confusion with two directory names.Rephrase to explicitly contrast guide vs. dummy app naming.
Suggested edit:
-> Example (dummy app): the configured components subdirectory is named `startup` instead of `ror_components`. +> Example (dummy app): the components subdirectory is `startup` (this guide’s examples use `ror_components`).
76-77: Minor grammar/style fix in the blockquote.Capitalize the sentence and hyphenate consistently.
-> Example (dummy app): see the server bundle entrypoint import. +> Example (dummy app): See the server-bundle entrypoint import.
164-166: Clarify what to remove vs. what to keep; fix helper wording.Steps 1–2 read as “remove everything,” then the layout adds one bundle. Specify that removals apply to auto-generated components only, and keep the explicit bundle for the excluded component. Also pluralize “helpers.”
-1. Remove generated entrypoints from parameters passed directly to `javascript_pack_tag` and `stylesheet_pack_tag`. -2. Remove generated entrypoints from parameters passed directly to `append_javascript_pack_tag` and `append_stylesheet_pack_tag`. +1. Remove auto‑generated component entrypoints from any explicit parameters passed to `javascript_pack_tag` and `stylesheet_pack_tag`. +2. Remove auto‑generated component entrypoints from any explicit parameters passed to the append helpers (`append_javascript_pack_tag`, `append_stylesheet_pack_tag`). Your layout would now contain: ```erb - <%= javascript_pack_tag('SpecialComponentNotToAutoLoadBundle') %> - <%= stylesheet_pack_tag('SpecialComponentNotToAutoLoadBundle') %> + <%# Only the excluded bundle remains explicit %> + <%= javascript_pack_tag 'SpecialComponentNotToAutoLoadBundle' %> + <%= stylesheet_pack_tag 'SpecialComponentNotToAutoLoadBundle' %> ```Additionally, consider using a consistent ERB style (with or without parentheses) across the doc.
Also applies to: 169-172
222-225: Mention TS extensions.Add that
.ts/.tsxare supported for client/server component files.-... name the component files `ComponentName.server.jsx` and `ComponentName.client.jsx`. +... name the component files `ComponentName.server.[jt]sx` and `ComponentName.client.[jt]sx`.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
docs/guides/file-system-based-automated-bundle-generation.md(7 hunks)
🧰 Additional context used
🧠 Learnings (5)
📚 Learning: 2025-04-26T21:55:55.874Z
Learnt from: alexeyr-ci2
PR: shakacode/react_on_rails#1732
File: spec/dummy/client/app-react16/startup/ReduxSharedStoreApp.client.jsx:40-44
Timestamp: 2025-04-26T21:55:55.874Z
Learning: In the react_on_rails project, files under `app-react16` directories are copied/moved to corresponding `/app` directories during the conversion process (removing the `-react16` suffix), which affects their relative import paths at runtime.
Applied to files:
docs/guides/file-system-based-automated-bundle-generation.md
📚 Learning: 2025-02-12T16:38:06.537Z
Learnt from: Romex91
PR: shakacode/react_on_rails#1697
File: package-scripts.yml:28-28
Timestamp: 2025-02-12T16:38:06.537Z
Learning: The file `node_package/lib/ReactOnRails.full.js` is autogenerated during the build process and should not be present in the repository.
Applied to files:
docs/guides/file-system-based-automated-bundle-generation.md
📚 Learning: 2024-10-08T20:53:47.076Z
Learnt from: theforestvn88
PR: shakacode/react_on_rails#1620
File: spec/dummy/client/app/startup/HelloTurboStream.jsx:3-3
Timestamp: 2024-10-08T20:53:47.076Z
Learning: The `RailsContext` import in `spec/dummy/client/app/startup/HelloTurboStream.jsx` is used later in the project, as clarified by the user theforestvn88.
Applied to files:
docs/guides/file-system-based-automated-bundle-generation.md
📚 Learning: 2025-02-13T19:09:15.991Z
Learnt from: AbanoubGhadban
PR: shakacode/react_on_rails#1644
File: node_package/src/RSCWebpackLoader.ts:0-0
Timestamp: 2025-02-13T19:09:15.991Z
Learning: In React Server Components webpack loader, using `new Function('return import("react-server-dom-webpack/node-loader")')()` is necessary as a workaround to bypass TypeScript compilation issues with direct dynamic imports.
Applied to files:
docs/guides/file-system-based-automated-bundle-generation.md
📚 Learning: 2025-07-08T05:57:29.630Z
Learnt from: AbanoubGhadban
PR: shakacode/react_on_rails#1745
File: node_package/src/RSCRequestTracker.ts:8-14
Timestamp: 2025-07-08T05:57:29.630Z
Learning: The global `generateRSCPayload` function in React on Rails Pro (RORP) is provided by the framework during rendering requests, not implemented in application code. The `declare global` statements are used to document the expected interface that RORP will inject at runtime.
Applied to files:
docs/guides/file-system-based-automated-bundle-generation.md
🪛 LanguageTool
docs/guides/file-system-based-automated-bundle-generation.md
[grammar] ~34-~34: There might be a mistake here.
Context: ...d startup instead of ror_components. > [Dummy initializer](https://github.com...
(QB_NEW_EN)
[grammar] ~76-~76: There might be a mistake here.
Context: ...see the server bundle entrypoint import. > [Dummy server-bundle.js](https://githu...
(QB_NEW_EN)
[grammar] ~164-~164: There might be a mistake here.
Context: ...ipt_pack_tagandstylesheet_pack_tag`. 2. Remove generated entrypoints from parame...
(QB_NEW_EN)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: dummy-app-integration-tests (oldest)
- GitHub Check: dummy-app-integration-tests (newest)
- GitHub Check: build
- GitHub Check: rspec-package-tests (newest)
- GitHub Check: rspec-package-tests (oldest)
🔇 Additional comments (4)
docs/guides/file-system-based-automated-bundle-generation.md (4)
47-49: Example note reads well.Accurately reflects the dummy app’s initializer setting.
160-163: Good clarification on the renamed example component.The note prevents readers from thinking they must rename files.
176-191: Good directory illustration.Clearly shows why the “SpecialComponent…” is not auto‑loaded by placing it outside the components subdirectory.
199-200: Example sequence is coherent.Explicit pack tag + react_component without auto_load_bundle is appropriate for the excluded component.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/guides/file-system-based-automated-bundle-generation.md (1)
121-127: Fix incorrect import paths for Bar components in the Background example.The
Barimports reference theFoofolder. This will mislead readers.import ReactOnRails from 'react-on-rails'; import FooComponentOne from '../src/Foo/FooComponentOne'; -import BarComponentOne from '../src/Foo/BarComponentOne'; -import BarComponentTwo from '../src/Foo/BarComponentTwo'; +import BarComponentOne from '../src/Bar/BarComponentOne'; +import BarComponentTwo from '../src/Bar/BarComponentTwo';
🧹 Nitpick comments (4)
docs/guides/file-system-based-automated-bundle-generation.md (4)
161-164: Clarify aliasing note to avoid reader confusion.Minor reword for flow.
-> Note: In the Background examples above, we used `BarComponentTwo`. In the Solution below, we refer to the same component as `SpecialComponentNotToAutoLoadBundle` to emphasize that it is excluded from auto-loading. You do not need to rename your files. +> Note: In the Background examples above, the component is named `BarComponentTwo`. In the Solution below, we refer to the same component as `SpecialComponentNotToAutoLoadBundle` to emphasize it is excluded from auto-loading. You do not need to rename your files.
171-173: Avoid double-including the same pack (layout vs. append).Later you append the same bundle in the view. Choose one approach to prevent duplication.
Option A (preferred): rely on per-view append, remove layout includes:
- <%= javascript_pack_tag('SpecialComponentNotToAutoLoadBundle') %> - <%= stylesheet_pack_tag('SpecialComponentNotToAutoLoadBundle') %> + <%# Intentionally left empty; per-view will append as needed %>Option B: keep layout includes; then remove the per-view append (see Lines 197-201 comment).
197-201: Make examples consistent: either append or include in layout, not both.If you adopt Option A (no layout include), keep the append and add the stylesheet append for parity. If you adopt Option B (layout includes), drop the append here.
Option A:
- <% append_javascript_pack_tag('SpecialComponentNotToAutoLoadBundle') %> + <% append_javascript_pack_tag('SpecialComponentNotToAutoLoadBundle') %> + <% append_stylesheet_pack_tag('SpecialComponentNotToAutoLoadBundle') %>Option B:
- <% append_javascript_pack_tag('SpecialComponentNotToAutoLoadBundle') %>
34-36: Tiny grammar/punctuation polish.Optional tweaks for smoother reading.
- > Example (dummy app): the configured components subdirectory is named `startup` instead of `ror_components`. + > Example (dummy app): the components subdirectory is `startup` (instead of `ror_components`). - > Example (dummy app): see the server bundle entrypoint import. + > Example (dummy app): see the server-bundle entrypoint import.Also applies to: 77-79
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
docs/guides/file-system-based-automated-bundle-generation.md(7 hunks)
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-04-26T21:55:55.874Z
Learnt from: alexeyr-ci2
PR: shakacode/react_on_rails#1732
File: spec/dummy/client/app-react16/startup/ReduxSharedStoreApp.client.jsx:40-44
Timestamp: 2025-04-26T21:55:55.874Z
Learning: In the react_on_rails project, files under `app-react16` directories are copied/moved to corresponding `/app` directories during the conversion process (removing the `-react16` suffix), which affects their relative import paths at runtime.
Applied to files:
docs/guides/file-system-based-automated-bundle-generation.md
📚 Learning: 2025-02-12T16:38:06.537Z
Learnt from: Romex91
PR: shakacode/react_on_rails#1697
File: package-scripts.yml:28-28
Timestamp: 2025-02-12T16:38:06.537Z
Learning: The file `node_package/lib/ReactOnRails.full.js` is autogenerated during the build process and should not be present in the repository.
Applied to files:
docs/guides/file-system-based-automated-bundle-generation.md
📚 Learning: 2024-10-08T20:53:47.076Z
Learnt from: theforestvn88
PR: shakacode/react_on_rails#1620
File: spec/dummy/client/app/startup/HelloTurboStream.jsx:3-3
Timestamp: 2024-10-08T20:53:47.076Z
Learning: The `RailsContext` import in `spec/dummy/client/app/startup/HelloTurboStream.jsx` is used later in the project, as clarified by the user theforestvn88.
Applied to files:
docs/guides/file-system-based-automated-bundle-generation.md
📚 Learning: 2025-02-13T19:09:15.991Z
Learnt from: AbanoubGhadban
PR: shakacode/react_on_rails#1644
File: node_package/src/RSCWebpackLoader.ts:0-0
Timestamp: 2025-02-13T19:09:15.991Z
Learning: In React Server Components webpack loader, using `new Function('return import("react-server-dom-webpack/node-loader")')()` is necessary as a workaround to bypass TypeScript compilation issues with direct dynamic imports.
Applied to files:
docs/guides/file-system-based-automated-bundle-generation.md
🪛 LanguageTool
docs/guides/file-system-based-automated-bundle-generation.md
[grammar] ~34-~34: There might be a mistake here.
Context: ...d startup instead of ror_components. > [Dummy initializer](https://github.com...
(QB_NEW_EN)
[grammar] ~77-~77: There might be a mistake here.
Context: ...see the server bundle entrypoint import. > [Dummy server-bundle.js](https://githu...
(QB_NEW_EN)
[grammar] ~165-~165: There might be a mistake here.
Context: ...ipt_pack_tagandstylesheet_pack_tag`. 2. Remove generated entrypoints from parame...
(QB_NEW_EN)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: dummy-app-integration-tests (newest)
- GitHub Check: dummy-app-integration-tests (oldest)
- GitHub Check: rspec-package-tests (newest)
- GitHub Check: rspec-package-tests (oldest)
- GitHub Check: build
🔇 Additional comments (6)
docs/guides/file-system-based-automated-bundle-generation.md (6)
20-22: Good cross-reference to dummy config.Accurate and helpful link-out; no action needed.
34-36: Initializer reference reads well.Clear pointer to the dummy app’s configured
components_subdirectory.
47-49: auto_load_bundle example is consistent with guidance.Matches the narrative; no change needed.
77-79: Server bundle import reference LGTM.Link and phrasing are clear.
180-192: Directory layout correctly separates entrypoint and source.Placing
SpecialComponentNotToAutoLoadBundle.jsxunderpacks(entry) and outsideror_components(source) aligns with the “do not auto-generate” intent.
225-226: Paired client/server file links are accurate and useful.Nice addition; matches current dummy app structure.
| ``` | ||
|
|
||
| 4. You no longer need to register the React components within the `ReactOnRails.configuration.components_subdirectory` nor directly add their bundles. For example, you can have a Rails view using three components: | ||
|
|
||
| ```erb | ||
| <% append_javascript_pack('BarComponentTwo') %> | ||
| <% append_javascript_pack_tag('SpecialComponentNotToAutoLoadBundle') %> | ||
| <%= react_component("FooComponentOne", {}, auto_load_bundle: true) %> |
There was a problem hiding this comment.
put line 200 right under this.
and should have a blank line separating
| @@ -38,6 +44,9 @@ You can change the value in `config/initializers/react_on_rails` by updating it | |||
| config.auto_load_bundle = true | |||
| ``` | |||
|
|
|||
| > Example (dummy app): `auto_load_bundle` is set to `true` in the same initializer. | |||
| > [Dummy initializer](https://github.com/shakacode/react_on_rails/blob/master/spec/dummy/config/initializers/react_on_rails.rb) | |||
There was a problem hiding this comment.
- Should all of the references have line numbers?
- Should they be on a particular commit? if we leave on master, line number will eventually be invalid.
I'll merge as-is for now.
There was a problem hiding this comment.
2. if we leave on master, line number will eventually be invalid.
yeah makes sense, i didn't see this one. i'll open a followup pr and address this
Summary
Closes #1756
Rename example “BarComponentTwo” to “SpecialComponentNotToAutoLoadBundle” in Solution section.
Add targeted cross-references to the dummy app (nested_entries, initializer, startup dir, server-bundle import).
Pull Request checklist
Remove this line after checking all the items here. If the item is not applicable to the PR, both check it out and wrap it by
~.[x]
Add/update test to cover these changes[x] Update documentation
[x]
Update CHANGELOG fileAdd the CHANGELOG entry at the top of the file.
Other Information
Verified behavior with a fresh demo app to ensure examples match expected outputs.
This change is
Summary by CodeRabbit