react_on_rails icon indicating copy to clipboard operation
react_on_rails copied to clipboard

Add Automated Files System Based Pack Generation

Open pulkitkkr opened this issue 2 years ago • 11 comments

Summary

This PR adds

  • react_on_rails:generate_packs rake task
  • updates to View Helpers to generate component packs if not present.
  • add auto_load_bundle config option
  • add tests for packs generation

Detail

It has been a mundane task to create entry files, e.g.: client_entry.js inside the packs directory for a long time. The sole purpose of this file was to import a component and register it using the ReactOnRails.register API for usage in rails views.

The PR automates this process by introducing the concept of react on rail components. Any .jsx file components inside the ror_components directory will be detected and registered automatically for the usage on Rails View using the new helper react_component_with_bundle.

The name of the directory can be configured in src/config/initializers/react_on_rails.rb by setting

config.components_directory = "new_directory_name"


This change is Reviewable

pulkitkkr avatar May 30 '22 22:05 pulkitkkr

@pulkitkkr what's remaining?

justin808 avatar Jun 15 '22 09:06 justin808

Needs documentation.

Needs tests.

Can we have the spec/dummy app show this?

justin808 avatar Jul 05 '22 21:07 justin808

@pulkitkkr what's remaining?

justin808 avatar Jul 07 '22 09:07 justin808

The tests look fine. If the .jsx files are supposed to be empty, maybe make it clear by making them consist of # Empty test component comment?

alexeyr-ci1 avatar Jul 12 '22 15:07 alexeyr-ci1

Also is load_bundle a good name for the added option? It doesn't look particularly related to components. But I am not certain what should replace it, if anything. load_component_bundle? load_component_pack?

alexeyr-ci1 avatar Jul 12 '22 15:07 alexeyr-ci1

@alexeyr-ci1 how about

auto_load_bundle

which signifies that no additional work is required to ensure the right bundles load on a given Rails view.

This works by automatically calling append_javascript_pack_tag and append_stylesheet_pack_tag whenever react_component and react_component_hash are used.

justin808 avatar Jul 12 '22 22:07 justin808

which signifies that no additional work is required to ensure the right bundles load on a given Rails view.

But then do we ever want it to be false? Or at least do we want it to be false by default?

Unless it's just for backward compatibility, in which case we can document that the default will be true in the next major version.

alexeyr-ci1 avatar Jul 14 '22 07:07 alexeyr-ci1

Address Docs related concerns @alexeyr

pulkitkkr avatar Jul 21 '22 21:07 pulkitkkr

https://github.com/shakacode/react_on_rails/blob/master/docs/guides/configuration.md says

Here is the full set of config options

so it should be updated too.

alexeyr-ci1 avatar Jul 22 '22 20:07 alexeyr-ci1

@pulkitkkr @alexeyr-ci1 Did you guys look in to the CI failures?

justin808 avatar Aug 01 '22 04:08 justin808

@pulkitkkr The errors just seem to be that tests expect an extra newline at the end of error messages. (I didn't check all of them, but those are all the ones I saw). I expect it's the tests which need to be fixed.

alexeyr-ci1 avatar Aug 01 '22 08:08 alexeyr-ci1

lib/react_on_rails/helper.rb line 99 at r13 (raw file):

Previously, justin808 (Justin Gordon) wrote…

What will happen in tests?

Can we be sure to have a clear error message?

The react on rails helper can do this, like it will do the i18 automatically.

See docs on the Spec helper https://github.com/shakacode/react_on_rails/blob/master/docs/guides/rspec-configuration.md

But some will use the compile option for Shakapacker and we need to be sure the error is clear.

Fixed it

pulkitkkr avatar Aug 16 '22 19:08 pulkitkkr

docs/guides/file-system-based-automated-bundle-generation.md line 110 at r21 (raw file):

Previously, justin808 (Justin Gordon) wrote…

We need to mention what's happening under the hood. I think that will make the transition clear from append_javascript_pack_tag, append_stylesheet_pack_tag to the new format.

I do not favour giving in-depth details about the internal functioning, as it will make it difficult for beginners to understand. Also, To understand the internals, the reader must be familiar with the shakapacker API. Think about someone who just started using React on Rails, following the example to configure and get created. We as creators are familiar with append_javascript_pack_tag and append_stylesheet_pack_tag; It's not something that many people will be familiar with.

If you still feel so, I can add details.

pulkitkkr avatar Aug 17 '22 02:08 pulkitkkr

Packs get generated after calling ReactOnRails::TestHelper.configure_rspec_to_compile_assets(config) in rails_helper.rb even though config.auto_load_bundle = false

https://github.com/shakacode/react_on_rails/blob/7b313944dc70680d874066431dc1e99e3b6935a2/lib/react_on_rails/test_helper/ensure_assets_compiled.rb#L40

In our case it breaks feature tests (an issue with sass variables, but there is no such an issue while running bin/webpacker), we don't need a pack for every component. I fixed our tests by adding:

module ReactOnRails
  class PacksGenerator
    def generate_packs_if_stale
      # does nothing
    end
  end
end

dnesteryuk avatar Feb 03 '23 14:02 dnesteryuk