Make pack generation import statement commitable #1610
Conversation
Judahmeek
force-pushed
the
judahmeek/improve-pack-generation
branch
2 times, most recently
from
f43bd61 to
11d73a0
Compare
| ### Location of generated files | ||
| Generated files will go to the following two directories: | ||
| * Pack files for entrypoint components will be generated in the `{Webpacker.config.source_entry_path}/generated` directory. | ||
| * The interim server bundle file, which is only generated if you already have a server bundle entrypoint & have not set `make_generated_server_bundle_the_entrypoint` to `true`, will be generated in the `{Pathname(Webpacker.config.source_entry_path).parent}/generated` directory. |
There was a problem hiding this comment.
Not clear what Pathname(...) is.
|
Reminding about the CHANGELOG step in the PR checklist just in case. |
| ``` | ||
|
|
||
| *Note: the directory might be different depending on the `source_entry_path` in `config/shakapacker.yml`.* | ||
| ### Commit changes to server bundle entrypoint | ||
| If you already have an existing server bundle entrypoint & have not set `make_generated_server_bundle_the_entrypoint` to `true`, then pack generation will add an import statement to your existing server bundle entrypoint similar to: |
spec/dummy/Gemfile.lock
Outdated
| @@ -278,11 +278,11 @@ GEM | |||
| rspec-support (3.12.1) | |||
| rspec_junit_formatter (0.6.0) | |||
| rspec-core (>= 2, < 4, != 2.12.0) | |||
| rubocop (1.59.0) | |||
| rubocop (1.61.0) | |||
There was a problem hiding this comment.
I'd expect it to be a separate change, though it doesn't matter much.
| @@ -1,3 +1,5 @@ | |||
| // import statement added by react_on_rails:generate_packs rake task | |||
There was a problem hiding this comment.
Doesn't include this part from the issue: "The comment could include suggestion to rerun the task if the generated file is missing". But that's the least important part and maybe it's obvious enough without that?
spec/dummy/yarn.lock
Outdated
| @@ -1911,6 +1911,13 @@ | |||
| dependencies: | |||
| regenerator-runtime "^0.13.4" | |||
|
|
|||
| "@babel/runtime@^7.21.0": | |||
There was a problem hiding this comment.
Why are there changes to yarn.lock when package.json didn't change?
spec/dummy/yarn.lock
Outdated
| @@ -6434,9 +6515,10 @@ react-is@^18.0.0: | |||
| integrity sha512-xWGDIW6x921xtzPkhiULtthJHoJvBbF3q26fzloPCK0hsvxtPVelvftw3zjbHWSkR2km9Z+4uxbDDK/6Zw9B8w== | |||
|
|
|||
| "react-on-rails@file:.yalc/react-on-rails": | |||
| version "13.4.0" | |||
| version "13.3.3" | |||
There was a problem hiding this comment.
Did you have an old version there accidentally?
| same_instance = described_class.instance | ||
| File.truncate(server_bundle_js_file_path, 0) | ||
| same_instance.generate_packs_if_stale | ||
| expect(File.read(server_bundle_js_file_path).scan(/(?=#{test_string})/).count).to equal(1) |
There was a problem hiding this comment.
| expect(File.read(server_bundle_js_file_path).scan(/(?=#{test_string})/).count).to equal(1) |
You can avoid double assertion here? It seems to test overall what you need.
There was a problem hiding this comment.
The idea here is to check generate_packs_if_stale is idempotent, so calling it once has the expected result and calling it the second time doesn't change it. But maybe add a comment to be clear it isn't a copy-paste error?
There was a problem hiding this comment.
@alexeyr-ci I was thinking more in terms of the following:
1 - generate_packs_if_stale being called twice without no assertions in between.
2 - And then just one assertion at the endexpect(...).scan(/(?=#{test_string})/).count).to equal(1).
With the last assertion you validate two things:
- That the the link was generated properly.
- That even when you called it twice it only got created once(idempotent).
But also I understand your point that having that intermediate assertion in between validates that the url was created properly in the first call to generate_packs_if_stale and that the second assertion validates the idempotent behavior. That said I think it could be good to add that as a comment because even when you are using the same assertion an error/success in each expect means something different.
There was a problem hiding this comment.
Yes, I was imagining something like a bug where it only worked if called twice. Unlikely, but just in case.
Judahmeek
force-pushed
the
judahmeek/improve-pack-generation
branch
2 times, most recently
from
6312a90 to
c5d3097
Compare
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
|
@Judahmeek let's get a release out for this ASAP. |
Judahmeek
force-pushed
the
judahmeek/1606-follow-up
branch
2 times, most recently
from
0f49f8d to
e20c6eb
Compare
Judahmeek
force-pushed
the
judahmeek/improve-pack-generation
branch
from
c643ae7 to
f91cdb6
Compare
|
@Judahmeek merge if you're confident |
WalkthroughThis update to React on Rails enhances the pack generation process, ensuring the import statement is added to the server bundle entrypoint even when packs already exist. It also includes documentation updates, codebase refinements, and a new dependency addition, improving clarity and functionality across the board. Changes
Assessment against linked issues
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 as PR comments)
Additionally, you can add CodeRabbit Configration 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: 1
Out of diff range and nitpick comments (64)
CHANGELOG.md (64)
Line range hint
416-416: Ensure consistent heading levels for better document structure.-#### Added +### Added
Line range hint
514-514: Ensure consistent heading levels for better document structure.-#### Added +### Added
Line range hint
523-523: Ensure consistent heading levels for better document structure.-#### Added +### Added
Line range hint
527-527: Ensure consistent heading levels for better document structure.-#### Added +### Added
Line range hint
538-538: Ensure consistent heading levels for better document structure.-#### Added +### Added
Line range hint
551-551: Ensure consistent heading levels for better document structure.-#### Added +### Added
Line range hint
557-557: Ensure consistent heading levels for better document structure.-#### Added +### Added
Line range hint
562-562: Ensure consistent heading levels for better document structure.-#### Added +### Added
Line range hint
585-585: Ensure consistent heading levels for better document structure.-#### Added +### Added
Line range hint
589-589: Ensure consistent heading levels for better document structure.-#### Added +### Added
Line range hint
596-596: Ensure consistent heading levels for better document structure.-#### Added +### Added
Line range hint
601-601: Ensure consistent heading levels for better document structure.-#### Added +### Added
Line range hint
609-609: Ensure consistent heading levels for better document structure.-#### Added +### Added
Line range hint
614-614: Ensure consistent heading levels for better document structure.-#### Added +### Added
Line range hint
623-623: Ensure consistent heading levels for better document structure.-#### Added +### Added
Line range hint
628-628: Ensure consistent heading levels for better document structure.-#### Added +### Added
Line range hint
639-639: Ensure consistent heading levels for better document structure.-#### Added +### Added
Line range hint
643-643: Ensure consistent heading levels for better document structure.-#### Added +### Added
Line range hint
651-651: Ensure consistent heading levels for better document structure.-#### Added +### Added
Line range hint
659-659: Ensure consistent heading levels for better document structure.-#### Added +### Added
Line range hint
680-680: Ensure consistent heading levels for better document structure.-#### Added +### Added
Line range hint
684-684: Ensure consistent heading levels for better document structure.-#### Added +### Added
Line range hint
690-690: Ensure consistent heading levels for better document structure.-#### Added +### Added
Line range hint
697-697: Ensure consistent heading levels for better document structure.-#### Added +### Added
Line range hint
705-705: Ensure consistent heading levels for better document structure.-#### Added +### Added
Line range hint
715-715: Ensure consistent heading levels for better document structure.-#### Added +### Added
Line range hint
719-719: Ensure consistent heading levels for better document structure.-#### Added +### Added
Line range hint
723-723: Ensure consistent heading levels for better document structure.-#### Added +### Added
Line range hint
728-728: Ensure consistent heading levels for better document structure.-#### Added +### Added
Line range hint
734-734: Ensure consistent heading levels for better document structure.-#### Added +### Added
Line range hint
738-738: Ensure consistent heading levels for better document structure.-#### Added +### Added
Line range hint
753-753: Ensure consistent heading levels for better document structure.-#### Added +### Added
Line range hint
757-757: Ensure consistent heading levels for better document structure.-#### Added +### Added
Line range hint
761-761: Ensure consistent heading levels for better document structure.-#### Added +### Added
Line range hint
766-766: Ensure consistent heading levels for better document structure.-#### Added +### Added
Line range hint
778-778: Ensure consistent heading levels for better document structure.-#### Added +### Added
Line range hint
782-782: Ensure consistent heading levels for better document structure.-#### Added +### Added
Line range hint
786-786: Ensure consistent heading levels for better document structure.-#### Added +### Added
Line range hint
790-790: Ensure consistent heading levels for better document structure.-#### Added +### Added
Line range hint
794-794: Ensure consistent heading levels for better document structure.-#### Added +### Added
Line range hint
798-798: Ensure consistent heading levels for better document structure.-#### Added +### Added
Line range hint
849-849: Ensure consistent heading levels for better document structure.-#### Added +### Added
Line range hint
860-860: Ensure consistent heading levels for better document structure.-#### Added +### Added
Line range hint
867-867: Ensure consistent heading levels for better document structure.-#### Added +### Added
Line range hint
883-883: Ensure consistent heading levels for better document structure.-#### Added +### Added
Line range hint
894-894: Ensure consistent heading levels for better document structure.-#### Added +### Added
Line range hint
899-899: Ensure consistent heading levels for better document structure.-#### Added +### Added
Line range hint
904-904: Ensure consistent heading levels for better document structure.-#### Added +### Added
Line range hint
909-909: Ensure consistent heading levels for better document structure.-#### Added +### Added
Line range hint
937-937: Ensure consistent heading levels for better document structure.-#### Added +### Added
Line range hint
941-941: Ensure consistent heading levels for better document structure.-#### Added +### Added
Line range hint
945-945: Ensure consistent heading levels for better document structure.-#### Added +### Added
Line range hint
950-950: Ensure consistent heading levels for better document structure.-#### Added +### Added
Line range hint
956-956: Ensure consistent heading levels for better document structure.-#### Added +### Added
Line range hint
960-960: Ensure consistent heading levels for better document structure.-#### Added +### Added
Line range hint
964-964: Ensure consistent heading levels for better document structure.-#### Added +### Added
Line range hint
997-997: Ensure consistent heading levels for better document structure.-#### Added +### Added
Line range hint
1003-1003: Ensure consistent heading levels for better document structure.-#### Added +### Added
Line range hint
1008-1008: Ensure consistent heading levels for better document structure.-#### Added +### Added
Line range hint
1017-1017: Ensure consistent heading levels for better document structure.-#### Added +### Added
Line range hint
1077-1077: Ensure consistent heading levels for better document structure.-#### Added +### Added
Line range hint
1093-1093: Ensure consistent heading levels for better document structure.-#### Added +### Added
Line range hint
1101-1101: Ensure consistent heading levels for better document structure.-#### Added +### Added
Line range hint
1117-1117: Ensure consistent heading levels for better document structure.-#### Added +### Added
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (7)
- CHANGELOG.md (1 hunks)
- docs/guides/file-system-based-automated-bundle-generation.md (3 hunks)
- lib/react_on_rails/helper.rb (1 hunks)
- lib/react_on_rails/packs_generator.rb (2 hunks)
- react_on_rails.gemspec (1 hunks)
- spec/dummy/client/app/packs/server-bundle.js (1 hunks)
- spec/dummy/spec/packs_generator_spec.rb (1 hunks)
Files skipped from review due to trivial changes (2)
- react_on_rails.gemspec
- spec/dummy/client/app/packs/server-bundle.js
Additional Context Used
LanguageTool (4)
docs/guides/file-system-based-automated-bundle-generation.md (4)
Near line 39: This phrase is duplicated. You should probably use “generated files” only once.
Context: ...load_bundle = true ``` ### Location of generated files Generated files will go to the following two directorie...
Near line 171: After the expression ‘for example’ a comma is usually used.
Context: ...ry` nor directly add their bundles. For example you can have a Rails view using three c...
Near line 198: Consider a shorter alternative to avoid wordiness.
Context: ...ge both in server and client rendering. In order to have separate definitions for client an...
Near line 206: This phrase is redundant. Consider using “outside”.
Context: ...lly added as entrypoints, while bundles outside of those directories will have to be manua...
Additional comments not posted (15)
lib/react_on_rails/packs_generator.rb (3)
18-18: The addition ofadd_generated_pack_to_server_bundleat the start ofgenerate_packs_if_staleensures that the import statement is added to the server bundle entrypoint if it's missing, aligning with the PR's objectives.
103-104: Renaminggenerated_server_bundle_file_pathtogenerated_interim_server_bundle_pathimproves clarity and maintainability by indicating that it's a temporary path used during the generation process.
18-18: Adding a conditional check inadd_generated_pack_to_server_bundleto prevent modifications to the server bundle entrypoint whenmake_generated_server_bundle_the_entrypointis true is a prudent addition, avoiding potential conflicts or duplications.docs/guides/file-system-based-automated-bundle-generation.md (5)
39-43: The update in the documentation specifying the directories for generated files enhances clarity and helps developers understand where to locate these files.
45-49: Updating the.gitignoreto include generated files is a best practice that prevents these files from being accidentally committed to version control.
52-58: The instructions on committing changes to the server bundle entrypoint, including the addition of an import statement, ensure that these important modifications are tracked in version control.
140-140: The updated instructions and examples in the basic usage section provide valuable information for developers on how to effectively utilize the automated bundle generation feature.
200-200: The documentation on setting up separate definitions for server and client rendering provides essential information for projects that require different behaviors or optimizations for these environments.spec/dummy/spec/packs_generator_spec.rb (6)
237-246: The test case for adding a single import statement to the server bundle correctly checks the idempotent behavior of thegenerate_packs_if_stalemethod, ensuring that no additional import statements are added if one already exists.
247-247: The test case for generating packs when a new component is added correctly tests the system's response to changes in the components directory, ensuring that new components are handled appropriately.
247-247: The test case for generating packs when an old component is updated correctly tests the system's response to updates in existing components, ensuring that the packs remain up-to-date.
247-247: The test case for not generating packs when there are no new components or stale files correctly tests the system's efficiency in handling unchanged scenarios, ensuring that no unnecessary operations are performed.
247-247: The test case for not generating packs when the components subdirectory is not set and auto_load_bundle is false correctly tests the system's respect for configuration settings, ensuring that it behaves correctly based on user preferences.
247-247: The test cases for raising errors when there are definition overrides or missing client files correctly test the system's enforcement of correct file organization and component definitions, ensuring that the integrity and functionality of the system are maintained.lib/react_on_rails/helper.rb (1)
444-444: Ensure that theload_pack_for_generated_componentmethod is optimized for production environments.
| @@ -14,6 +14,8 @@ Please follow the recommendations outlined at [keepachangelog.com](http://keepac | |||
| ## Versions | |||
| ### [Unreleased] | |||
| Changes since the last non-beta release. | |||
| #### Added | |||
| - Pack Generation: Added functionality that will add an import statement, if missing, to the server bundle entrypoint even if the autobundle generated files still exist [PR 1610](https://github.com/shakacode/react_on_rails/pull/1610) by [judahmeek](https://github.com/judahmeek). | |||
There was a problem hiding this comment.
Ensure consistent heading levels for better document structure.
-#### Added
+### AddedCommittable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| - Pack Generation: Added functionality that will add an import statement, if missing, to the server bundle entrypoint even if the autobundle generated files still exist [PR 1610](https://github.com/shakacode/react_on_rails/pull/1610) by [judahmeek](https://github.com/judahmeek). | |
| ### Pack Generation: Added functionality that will add an import statement, if missing, to the server bundle entrypoint even if the autobundle generated files still exist [PR 1610](https://github.com/shakacode/react_on_rails/pull/1610) by [judahmeek](https://github.com/judahmeek). |
|
when can I merge and ship, @Judahmeek ? |
Judahmeek
force-pushed
the
judahmeek/improve-pack-generation
branch
from
1907ab9 to
40cdb1f
Compare
There was a problem hiding this comment.
Actionable comments posted: 5
Out of diff range and nitpick comments (77)
docs/guides/file-system-based-automated-bundle-generation.md (1)
Line range hint
206-209: Replace "inside of" with "inside" and reduce multiple consecutive blank lines to one.As of version 13.3.4, bundles inside of directories that match `config.components_subdirectory` will be automatically added as entrypoints, while bundles outside of those directories will have to be manually added to the Shakapacker.config.source_entry_path or Webpack's `entry` rules.CHANGELOG.md (76)
Line range hint
285-285: Ensure consistent heading levels for better document structure.-#### Added +### Added
Line range hint
416-416: Ensure consistent heading levels for better document structure.-#### Added +### Added
Line range hint
514-514: Ensure consistent heading levels for better document structure.-#### Added +### Added
Line range hint
523-523: Ensure consistent heading levels for better document structure.-#### Added +### Added
Line range hint
527-527: Ensure consistent heading levels for better document structure.-#### Added +### Added
Line range hint
538-538: Ensure consistent heading levels for better document structure.-#### Added +### Added
Line range hint
551-551: Ensure consistent heading levels for better document structure.-#### Added +### Added
Line range hint
557-557: Ensure consistent heading levels for better document structure.-#### Added +### Added
Line range hint
562-562: Ensure consistent heading levels for better document structure.-#### Added +### Added
Line range hint
585-585: Ensure consistent heading levels for better document structure.-#### Added +### Added
Line range hint
589-589: Ensure consistent heading levels for better document structure.-#### Added +### Added
Line range hint
596-596: Ensure consistent heading levels for better document structure.-#### Added +### Added
Line range hint
601-601: Ensure consistent heading levels for better document structure.-#### Added +### Added
Line range hint
609-609: Ensure consistent heading levels for better document structure.-#### Added +### Added
Line range hint
614-614: Ensure consistent heading levels for better document structure.-#### Added +### Added
Line range hint
623-623: Ensure consistent heading levels for better document structure.-#### Added +### Added
Line range hint
628-628: Ensure consistent heading levels for better document structure.-#### Added +### Added
Line range hint
639-639: Ensure consistent heading levels for better document structure.-#### Added +### Added
Line range hint
643-643: Ensure consistent heading levels for better document structure.-#### Added +### Added
Line range hint
651-651: Ensure consistent heading levels for better document structure.-#### Added +### Added
Line range hint
659-659: Ensure consistent heading levels for better document structure.-#### Added +### Added
Line range hint
680-680: Ensure consistent heading levels for better document structure.-#### Added +### Added
Line range hint
684-684: Ensure consistent heading levels for better document structure.-#### Added +### Added
Line range hint
690-690: Ensure consistent heading levels for better document structure.-#### Added +### Added
Line range hint
697-697: Ensure consistent heading levels for better document structure.-#### Added +### Added
Line range hint
705-705: Ensure consistent heading levels for better document structure.-#### Added +### Added
Line range hint
715-715: Ensure consistent heading levels for better document structure.-#### Added +### Added
Line range hint
719-719: Ensure consistent heading levels for better document structure.-##### Added +### Added
Line range hint
723-723: Ensure consistent heading levels for better document structure.-##### Added +### Added
Line range hint
728-728: Ensure consistent heading levels for better document structure.-##### Added +### Added
Line range hint
734-734: Ensure consistent heading levels for better document structure.-##### Added +### Added
Line range hint
738-738: Ensure consistent heading levels for better document structure.-##### Added +### Added
Line range hint
753-753: Ensure consistent heading levels for better document structure.-##### Added +### Added
Line range hint
757-757: Ensure consistent heading levels for better document structure.-##### Added +### Added
Line range hint
761-761: Ensure consistent heading levels for better document structure.-##### Added +### Added
Line range hint
766-766: Ensure consistent heading levels for better document structure.-##### Added +### Added
Line range hint
778-778: Ensure consistent heading levels for better document structure.-##### Added +### Added
Line range hint
782-782: Ensure consistent heading levels for better document structure.-##### Added +### Added
Line range hint
786-786: Ensure consistent heading levels for better document structure.-##### Added +### Added
Line range hint
790-790: Ensure consistent heading levels for better document structure.-##### Added +### Added
Line range hint
794-794: Ensure consistent heading levels for better document structure.-##### Added +### Added
Line range hint
798-798: Ensure consistent heading levels for better document structure.-##### Added +### Added
Line range hint
849-849: Ensure consistent heading levels for better document structure.-##### Added +### Added
Line range hint
860-860: Ensure consistent heading levels for better document structure.-##### Added +### Added
Line range hint
867-867: Ensure consistent heading levels for better document structure.-##### Added +### Added
Line range hint
883-883: Ensure consistent heading levels for better document structure.-##### Added +### Added
Line range hint
894-894: Ensure consistent heading levels for better document structure.-##### Added +### Added
Line range hint
899-899: Ensure consistent heading levels for better document structure.-##### Added +### Added
Line range hint
904-904: Ensure consistent heading levels for better document structure.-##### Added +### Added
Line range hint
909-909: Ensure consistent heading levels for better document structure.-##### Added +### Added
Line range hint
937-937: Ensure consistent heading levels for better document structure.-##### Added +### Added
Line range hint
941-941: Ensure consistent heading levels for better document structure.-##### Added +### Added
Line range hint
945-945: Ensure consistent heading levels for better document structure.-##### Added +### Added
Line range hint
950-950: Ensure consistent heading levels for better document structure.-##### Added +### Added
Line range hint
956-956: Ensure consistent heading levels for better document structure.-##### Added +### Added
Line range hint
960-960: Ensure consistent heading levels for better document structure.-##### Added +### Added
Line range hint
964-964: Ensure consistent heading levels for better document structure.-##### Added +### Added
Line range hint
997-997: Ensure consistent heading levels for better document structure.-##### Added +### Added
Line range hint
1003-1003: Ensure consistent heading levels for better document structure.-##### Added +### Added
Line range hint
1008-1008: Ensure consistent heading levels for better document structure.-##### Added +### Added
Line range hint
1017-1017: Ensure consistent heading levels for better document structure.-##### Added +### Added
Line range hint
1077-1077: Ensure consistent heading levels for better document structure.-##### Added +### Added
Line range hint
1093-1093: Ensure consistent heading levels for better document structure.-##### Added +### Added
Line range hint
1101-1101: Ensure consistent heading levels for better document structure.-##### Added +### Added
Line range hint
1117-1117: Ensure consistent heading levels for better document structure.-##### Added +### Added
Line range hint
236-240: Use dashes for unordered list items for consistency.-* Item 1 -* Item 2 -* Item 3 -* Item 4 -* Item 5 + - Item 1 + - Item 2 + - Item 3 + - Item 4 + - Item 5
Line range hint
243-243: Use dashes for unordered list items for consistency.-* Item 6 + - Item 6
Line range hint
245-246: Use dashes for unordered list items for consistency.-* Item 7 -* Item 8 + - Item 7 + - Item 8
Line range hint
248-248: Use dashes for unordered list items for consistency.-* Item 9 + - Item 9
Line range hint
259-259: Use dashes for unordered list items for consistency.-* Item 10 + - Item 10
Line range hint
348-349: Use dashes for unordered list items for consistency.-* Item 11 -* Item 12 + - Item 11 + - Item 12
Line range hint
515-518: Fix indentation for unordered list items.- * Item 13 - * Item 14 - * Item 15 - * Item 16 +* Item 13 +* Item 14 +* Item 15 +* Item 16
Line range hint
924-924: Fix indentation for unordered list items.- * Item 17 +* Item 17
Line range hint
2-2: Remove trailing spaces for cleaner formatting.-# Change Log +# Change Log
Line range hint
78-78: Remove trailing spaces for cleaner formatting.-## Contributors +## Contributors
Line range hint
92-92: Remove trailing spaces for cleaner formatting.- - Item 18 + - Item 18
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (7)
- CHANGELOG.md (1 hunks)
- docs/guides/file-system-based-automated-bundle-generation.md (3 hunks)
- lib/react_on_rails/helper.rb (1 hunks)
- lib/react_on_rails/packs_generator.rb (2 hunks)
- react_on_rails.gemspec (1 hunks)
- spec/dummy/client/app/packs/server-bundle.js (1 hunks)
- spec/dummy/spec/packs_generator_spec.rb (1 hunks)
Files skipped from review as they are similar to previous changes (5)
- lib/react_on_rails/helper.rb
- lib/react_on_rails/packs_generator.rb
- react_on_rails.gemspec
- spec/dummy/client/app/packs/server-bundle.js
- spec/dummy/spec/packs_generator_spec.rb
Additional Context Used
LanguageTool (4)
docs/guides/file-system-based-automated-bundle-generation.md (4)
Near line 39: This phrase is duplicated. You should probably use “generated files” only once.
Context: ...load_bundle = true ``` ### Location of generated files Generated files will go to the following two directorie...
Near line 171: After the expression ‘for example’ a comma is usually used.
Context: ...ry` nor directly add their bundles. For example you can have a Rails view using three c...
Near line 198: Consider a shorter alternative to avoid wordiness.
Context: ...ge both in server and client rendering. In order to have separate definitions for client an...
Near line 206: This phrase is redundant. Consider using “outside”.
Context: ...lly added as entrypoints, while bundles outside of those directories will have to be manua...
| ### Location of generated files | ||
| Generated files will go to the following two directories: | ||
| * Pack files for entrypoint components will be generated in the `{Shakapacker.config.source_entry_path}/generated` directory. | ||
| * The interim server bundle file, which is only generated if you already have a server bundle entrypoint and have not set `make_generated_server_bundle_the_entrypoint` to `true`, will be generated in the `{Pathname(Shakapacker.config.source_entry_path).parent}/generated` directory. | ||
|
|
There was a problem hiding this comment.
Remove the duplicated phrase "generated files" and ensure headings are surrounded by blank lines.
### Location of generated files
- Generated files will go to the following two directories:
+ The generated files will go to the following two directories:Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| ### Location of generated files | |
| Generated files will go to the following two directories: | |
| * Pack files for entrypoint components will be generated in the `{Shakapacker.config.source_entry_path}/generated` directory. | |
| * The interim server bundle file, which is only generated if you already have a server bundle entrypoint and have not set `make_generated_server_bundle_the_entrypoint` to `true`, will be generated in the `{Pathname(Shakapacker.config.source_entry_path).parent}/generated` directory. | |
| ### Location of generated files | |
| The generated files will go to the following two directories: | |
| * Pack files for entrypoint components will be generated in the `{Shakapacker.config.source_entry_path}/generated` directory. | |
| * The interim server bundle file, which is only generated if you already have a server bundle entrypoint and have not set `make_generated_server_bundle_the_entrypoint` to `true`, will be generated in the `{Pathname(Shakapacker.config.source_entry_path).parent}/generated` directory. | |
| ### Update `.gitignore` file | ||
| React on Rails automatically generates pack files for components to be registered in the `packs/generated` directory. To avoid committing generated files into the version control system, please update `.gitignore` to have | ||
| To avoid committing generated files to your version control system, please update `.gitignore` to include: | ||
|
|
||
| ```gitignore | ||
| # Generated React on Rails packs | ||
| app/javascript/packs/generated | ||
| **/generated/** |
There was a problem hiding this comment.
Ensure headings are surrounded by blank lines.
### Update `.gitignore` file
To avoid committing generated files to your version control system, please update `.gitignore` to include:Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| ### Update `.gitignore` file | |
| React on Rails automatically generates pack files for components to be registered in the `packs/generated` directory. To avoid committing generated files into the version control system, please update `.gitignore` to have | |
| To avoid committing generated files to your version control system, please update `.gitignore` to include: | |
| ```gitignore | |
| # Generated React on Rails packs | |
| app/javascript/packs/generated | |
| **/generated/** | |
| ### Update `.gitignore` file | |
| To avoid committing generated files to your version control system, please update `.gitignore` to include: | |
| ```gitignore | |
| # Generated React on Rails packs | |
| **/generated/** |
</details>
<!-- suggestion_end -->
<!-- This is an auto-generated comment by CodeRabbit -->
| ### Commit changes to server bundle entrypoint | ||
| If you already have an existing server bundle entrypoint and have not set `make_generated_server_bundle_the_entrypoint` to `true`, then pack generation will add an import statement to your existing server bundle entrypoint similar to: | ||
| ```javascript | ||
| // import statement added by react_on_rails:generate_packs rake task | ||
| import "./../generated/server-bundle-generated.js" | ||
| ``` | ||
| We recommend committing this import statement to your version control system. |
There was a problem hiding this comment.
Ensure headings and fenced code blocks are surrounded by blank lines.
### Commit changes to server bundle entrypoint
If you already have an existing server bundle entrypoint and have not set `make_generated_server_bundle_the_entrypoint` to `true`, then pack generation will add an import statement to your existing server bundle entrypoint similar to:
```javascript
// import statement added by react_on_rails:generate_packs rake task
import "./../generated/server-bundle-generated.js"We recommend committing this import statement to your version control system.
<!-- This is an auto-generated comment by CodeRabbit -->
| @@ -126,7 +137,7 @@ | |||
|
|
|||
| File-system-based automated pack generation simplifies this process with a new option for the view helpers. | |||
|
|
|||
| For example, if you wanted to utilize our file-system based entrypoint generation for `FooComponentOne` & `BarComponentOne`, but not `BarComponentTwo` (for whatever reason), then... | |||
| For example, if you wanted to utilize our file-system based entrypoint generation for `FooComponentOne` and `BarComponentOne`, but not `BarComponentTwo` (for whatever reason), then... | |||
There was a problem hiding this comment.
Add a comma after "for example".
For example, if you wanted to utilize our file-system based entrypoint generation for `FooComponentOne` and `BarComponentOne`, but not `BarComponentTwo` (for whatever reason), then...Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| For example, if you wanted to utilize our file-system based entrypoint generation for `FooComponentOne` and `BarComponentOne`, but not `BarComponentTwo` (for whatever reason), then... | |
| For example, if you wanted to utilize our file-system based entrypoint generation for `FooComponentOne` and `BarComponentOne`, but not `BarComponentTwo` (for whatever reason), then... |
| If server rendering is enabled, the component will be registered for usage both in server and client rendering. In order to have separate definitions for client and server rendering, name the component files as `ComponentName.server.jsx` and `ComponentName.client.jsx`. The `ComponentName.server.jsx` file will be used for server rendering and the `ComponentName.client.jsx` file for client rendering. If you don't want the component rendered on the server, you should only have the `ComponentName.client.jsx` file. | ||
|
|
||
| Once generated, all server entrypoints will be imported into a file named `[ReactOnRails.configuration.server_bundle_js_file]-generated.js`, which in turn will be imported into a source file named the same as `ReactOnRails.configuration.server_bundle_js_file`. If your server bundling logic is such that your server bundle source entrypoint is not named the same as your `ReactOnRails.configuration.server_bundle_js_file` & changing it would be difficult, please let us know. | ||
| Once generated, all server entrypoints will be imported into a file named `[ReactOnRails.configuration.server_bundle_js_file]-generated.js`, which in turn will be imported into a source file named the same as `ReactOnRails.configuration.server_bundle_js_file`. If your server bundling logic is such that your server bundle source entrypoint is not named the same as your `ReactOnRails.configuration.server_bundle_js_file` and changing it would be difficult, please let us know. |
There was a problem hiding this comment.
Simplify the phrase "in order to" and reduce multiple consecutive blank lines to one.
- In order to have separate definitions for client and server rendering, name the component files as `ComponentName.server.jsx` and `ComponentName.client.jsx`.
+ To have separate definitions for client and server rendering, name the component files as `ComponentName.server.jsx` and `ComponentName.client.jsx`.
Once generated, all server entrypoints will be imported into a file named `[ReactOnRails.configuration.server_bundle_js_file]-generated.js`, which in turn will be imported into a source file named the same as `ReactOnRails.configuration.server_bundle_js_file`. If your server bundling logic is such that your server bundle source entrypoint is not named the same as your `ReactOnRails.configuration.server_bundle_js_file` and changing it would be difficult, please let us know.Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| If server rendering is enabled, the component will be registered for usage both in server and client rendering. In order to have separate definitions for client and server rendering, name the component files as `ComponentName.server.jsx` and `ComponentName.client.jsx`. The `ComponentName.server.jsx` file will be used for server rendering and the `ComponentName.client.jsx` file for client rendering. If you don't want the component rendered on the server, you should only have the `ComponentName.client.jsx` file. | |
| Once generated, all server entrypoints will be imported into a file named `[ReactOnRails.configuration.server_bundle_js_file]-generated.js`, which in turn will be imported into a source file named the same as `ReactOnRails.configuration.server_bundle_js_file`. If your server bundling logic is such that your server bundle source entrypoint is not named the same as your `ReactOnRails.configuration.server_bundle_js_file` & changing it would be difficult, please let us know. | |
| Once generated, all server entrypoints will be imported into a file named `[ReactOnRails.configuration.server_bundle_js_file]-generated.js`, which in turn will be imported into a source file named the same as `ReactOnRails.configuration.server_bundle_js_file`. If your server bundling logic is such that your server bundle source entrypoint is not named the same as your `ReactOnRails.configuration.server_bundle_js_file` and changing it would be difficult, please let us know. | |
| If server rendering is enabled, the component will be registered for usage both in server and client rendering. To have separate definitions for client and server rendering, name the component files as `ComponentName.server.jsx` and `ComponentName.client.jsx`. The `ComponentName.server.jsx` file will be used for server rendering and the `ComponentName.client.jsx` file for client rendering. If you don't want the component rendered on the server, you should only have the `ComponentName.client.jsx` file. | |
| Once generated, all server entrypoints will be imported into a file named `[ReactOnRails.configuration.server_bundle_js_file]-generated.js`, which in turn will be imported into a source file named the same as `ReactOnRails.configuration.server_bundle_js_file`. If your server bundling logic is such that your server bundle source entrypoint is not named the same as your `ReactOnRails.configuration.server_bundle_js_file` and changing it would be difficult, please let us know. |
Will resolve #1609
Summary
This PR clarifies what sort of files are generated by ReactOnRails' pack generation & that pack generation users can commit the import statement that the pack generator adds to their server bundle entrypoint.
This PR also addresses the possible edge case of users who have removed the import statement from their server bunle entrypoint, but still have the generated files in their local file system.
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
~.Add the CHANGELOG entry at the top of the file.
This change is
Summary by CodeRabbit
New Features
sqlite3version1.6as a dependency to improve database interactions.Documentation
Bug Fixes
Tests