From ac3bb3fbb8c09a9812ebfd2d6742ba37680e6813 Mon Sep 17 00:00:00 2001 From: Gannon McGibbon Date: Wed, 19 Aug 2020 18:54:14 -0400 Subject: [PATCH 1/2] Allow engine mounting to be disabled --- gems/quilt_rails/CHANGELOG.md | 5 +++ gems/quilt_rails/lib/quilt_rails.rb | 3 +- .../lib/quilt_rails/configuration.rb | 21 +++++++---- gems/quilt_rails/lib/quilt_rails/engine.rb | 4 ++- .../test/controllers/routing_test.rb | 19 ++++++++-- .../test/quilt_rails/configuration_test.rb | 35 ++++++++++++------- 6 files changed, 64 insertions(+), 23 deletions(-) diff --git a/gems/quilt_rails/CHANGELOG.md b/gems/quilt_rails/CHANGELOG.md index c76b0a9fef..bf59a83307 100644 --- a/gems/quilt_rails/CHANGELOG.md +++ b/gems/quilt_rails/CHANGELOG.md @@ -7,6 +7,11 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. +### Added + +- Added `config.mount` (default `true`) option to disable automatic engine mounting. + ([#1605](https://github.com/Shopify/quilt/pull/1605)) + ## [3.3.0] - 2020-08-10 ### Added diff --git a/gems/quilt_rails/lib/quilt_rails.rb b/gems/quilt_rails/lib/quilt_rails.rb index be71f0fbfb..039d830283 100644 --- a/gems/quilt_rails/lib/quilt_rails.rb +++ b/gems/quilt_rails/lib/quilt_rails.rb @@ -3,11 +3,12 @@ module Quilt end require "quilt_rails/version" -require "quilt_rails/engine" require "quilt_rails/logger" require "quilt_rails/configuration" require "quilt_rails/react_renderable" require "quilt_rails/performance" require "quilt_rails/trusted_ui_server_csrf_strategy" require "quilt_rails/header_csrf_strategy" + +require "quilt_rails/engine" require "quilt_rails/monkey_patches/active_support_reloader" if Rails.env.development? diff --git a/gems/quilt_rails/lib/quilt_rails/configuration.rb b/gems/quilt_rails/lib/quilt_rails/configuration.rb index 495a718962..2dc25378bb 100644 --- a/gems/quilt_rails/lib/quilt_rails/configuration.rb +++ b/gems/quilt_rails/lib/quilt_rails/configuration.rb @@ -1,18 +1,25 @@ # frozen_string_literal: true -module Quilt - class Configuration - attr_accessor :react_server_host, :react_server_protocol +require "active_support/ordered_options" +module Quilt + class Configuration < ActiveSupport::OrderedOptions def initialize - ip = ENV['REACT_SERVER_IP'] || 'localhost' - port = ENV['REACT_SERVER_PORT'] || 8081 + super + react_server_ip = ENV['REACT_SERVER_IP'] || "localhost" + react_server_port = ENV['REACT_SERVER_PORT'] || 8081 - @react_server_host = "#{ip}:#{port}" - @react_server_protocol = ENV['REACT_SERVER_PROTOCOL'] || 'http' + self.react_server_host = "#{react_server_ip}:#{react_server_port}" + self.react_server_protocol = ENV['REACT_SERVER_PROTOCOL'] || "http" + self.mount = true + end + + def mount? + mount end end + def self.configuration @configuration ||= Configuration.new end diff --git a/gems/quilt_rails/lib/quilt_rails/engine.rb b/gems/quilt_rails/lib/quilt_rails/engine.rb index 4d4e3d72aa..501498f1ce 100644 --- a/gems/quilt_rails/lib/quilt_rails/engine.rb +++ b/gems/quilt_rails/lib/quilt_rails/engine.rb @@ -4,10 +4,12 @@ module Quilt class Engine < ::Rails::Engine isolate_namespace Quilt + config.quilt = Quilt.configuration + initializer(:mount_quilt, before: :add_builtin_route) do |app| app.routes.append do mount(Quilt::Engine, at: '/') unless has_named_route?(:quilt) - end + end if config.quilt.mount? end end end diff --git a/gems/quilt_rails/test/controllers/routing_test.rb b/gems/quilt_rails/test/controllers/routing_test.rb index 62d6342efe..46b2b4abef 100644 --- a/gems/quilt_rails/test/controllers/routing_test.rb +++ b/gems/quilt_rails/test/controllers/routing_test.rb @@ -10,9 +10,9 @@ class RoutingTest < ActionDispatch::IntegrationTest class PerformanceReportController < ActionController::Base; end class UiController < ActionController::Base; end - setup { boot_dummy } - test "routes on app without routes" do + boot_dummy + assert_recognizes( { controller: "quilt/performance_report", action: "create" }, { path: "/performance_report", method: "post" } @@ -29,6 +29,8 @@ class UiController < ActionController::Base; end end test "routes on app with engine mount" do + boot_dummy + Rails.application.routes.draw do mount(Quilt::Engine, at: '/quilt') end @@ -49,6 +51,8 @@ class UiController < ActionController::Base; end end test "routes on app with custom controllers" do + boot_dummy + Rails.application.routes.draw do post '/performance_report', to: 'quilt/routing_test/performance_report#create' get '/*path', to: 'quilt/routing_test/ui#index' @@ -69,6 +73,17 @@ class UiController < ActionController::Base; end ) end + test "routes on app with mounting disabled" do + Quilt.configuration.mount = false + + boot_dummy + + assert_recognizes( + { controller: "rails/welcome", action: "index" }, + { path: "/" } + ) + end + private def boot_dummy diff --git a/gems/quilt_rails/test/quilt_rails/configuration_test.rb b/gems/quilt_rails/test/quilt_rails/configuration_test.rb index 7fc420a268..9005d80460 100644 --- a/gems/quilt_rails/test/quilt_rails/configuration_test.rb +++ b/gems/quilt_rails/test/quilt_rails/configuration_test.rb @@ -4,41 +4,52 @@ module Quilt class ConfigurationTest < Minitest::Test def setup - @initial_react_server_host = Quilt.configuration.react_server_host - @initial_react_server_protocol = Quilt.configuration.react_server_protocol + @original_configuration = Quilt.configuration + Quilt.instance_variable_set(:@configuration, Quilt::Configuration.new) end def teardown - Quilt.configuration.react_server_host = @initial_react_server_host - Quilt.configuration.react_server_protocol = @initial_react_server_protocol + Quilt.instance_variable_set(:@configuration, @original_configuration) end - def test_react_server_host_defaults + def test_react_server_host_default assert_equal('localhost:8081', Quilt.configuration.react_server_host) end - def test_allows_react_server_host_to_be_configured + def test_react_server_host_configured url = 'localhost:2222' - Quilt.configure do |configuration| - configuration.react_server_host = url + Quilt.configure do |config| + config.react_server_host = url end assert_equal(url, Quilt.configuration.react_server_host) end - def test_react_server_protocol_defaults + def test_react_server_protocol_default assert_equal('http', Quilt.configuration.react_server_protocol) end - def test_allows_react_server_protocol_to_be_configured + def test_react_server_protocol_configured protocol = 'https' - Quilt.configure do |configuration| - configuration.react_server_protocol = protocol + Quilt.configure do |config| + config.react_server_protocol = protocol end assert_equal(protocol, Quilt.configuration.react_server_protocol) end + + def test_mount_default + assert_predicate(Quilt.configuration, :mount?) + end + + def test_mount_configured + Quilt.configure do |config| + config.mount = false + end + + refute_predicate(Quilt.configuration, :mount?) + end end end From c3d4c4b8a4ef21e28542c46d4a100b73253b87c0 Mon Sep 17 00:00:00 2001 From: Gannon McGibbon Date: Thu, 20 Aug 2020 17:59:15 -0400 Subject: [PATCH 2/2] Fix warnings in test runs, rubocop --- ...ify-github-io-ruby-style-guide-rubocop-yml | 1021 ----------------- gems/quilt_rails/.rubocop.yml | 27 +- gems/quilt_rails/Gemfile | 2 + gems/quilt_rails/Gemfile.lock | 44 +- gems/quilt_rails/Rakefile | 2 + .../quilt/performance_report_controller.rb | 4 +- .../rails_setup/rails_setup_generator.rb | 8 +- .../react_setup/react_setup_generator.rb | 2 +- .../lib/quilt_rails/configuration.rb | 1 - gems/quilt_rails/lib/quilt_rails/engine.rb | 8 +- .../lib/quilt_rails/header_csrf_strategy.rb | 7 +- .../lib/quilt_rails/performance/report.rb | 36 +- .../lib/quilt_rails/react_renderable.rb | 14 +- 13 files changed, 84 insertions(+), 1092 deletions(-) delete mode 100644 gems/quilt_rails/.rubocop-http---shopify-github-io-ruby-style-guide-rubocop-yml diff --git a/gems/quilt_rails/.rubocop-http---shopify-github-io-ruby-style-guide-rubocop-yml b/gems/quilt_rails/.rubocop-http---shopify-github-io-ruby-style-guide-rubocop-yml deleted file mode 100644 index 516cb11603..0000000000 --- a/gems/quilt_rails/.rubocop-http---shopify-github-io-ruby-style-guide-rubocop-yml +++ /dev/null @@ -1,1021 +0,0 @@ -AllCops: - Exclude: - - 'db/schema.rb' - DisabledByDefault: true - StyleGuideBaseURL: https://shopify.github.io/ruby-style-guide/ - -Lint/AssignmentInCondition: - Enabled: true - -Layout/AccessModifierIndentation: - EnforcedStyle: indent - SupportedStyles: - - outdent - - indent - IndentationWidth: - -Style/Alias: - EnforcedStyle: prefer_alias_method - SupportedStyles: - - prefer_alias - - prefer_alias_method - -Layout/HashAlignment: - EnforcedHashRocketStyle: key - EnforcedColonStyle: key - EnforcedLastArgumentHashStyle: ignore_implicit - SupportedLastArgumentHashStyles: - - always_inspect - - always_ignore - - ignore_implicit - - ignore_explicit - -Layout/ParameterAlignment: - EnforcedStyle: with_fixed_indentation - SupportedStyles: - - with_first_parameter - - with_fixed_indentation - IndentationWidth: - -Style/AndOr: - EnforcedStyle: always - SupportedStyles: - - always - - conditionals - -Style/BarePercentLiterals: - EnforcedStyle: bare_percent - SupportedStyles: - - percent_q - - bare_percent - -Style/BlockDelimiters: - EnforcedStyle: line_count_based - SupportedStyles: - - line_count_based - - semantic - - braces_for_chaining - ProceduralMethods: - - benchmark - - bm - - bmbm - - create - - each_with_object - - measure - - new - - realtime - - tap - - with_object - FunctionalMethods: - - let - - let! - - subject - - watch - IgnoredMethods: - - lambda - - proc - - it - -Layout/CaseIndentation: - EnforcedStyle: end - SupportedStyles: - - case - - end - IndentOneStep: false - IndentationWidth: - -Style/ClassAndModuleChildren: - EnforcedStyle: nested - SupportedStyles: - - nested - - compact - -Style/ClassCheck: - EnforcedStyle: is_a? - SupportedStyles: - - is_a? - - kind_of? - -Style/CommandLiteral: - EnforcedStyle: percent_x - SupportedStyles: - - backticks - - percent_x - - mixed - AllowInnerBackticks: false - -Style/CommentAnnotation: - Keywords: - - TODO - - FIXME - - OPTIMIZE - - HACK - - REVIEW - -Style/ConditionalAssignment: - EnforcedStyle: assign_to_condition - SupportedStyles: - - assign_to_condition - - assign_inside_condition - SingleLineConditionsOnly: true - -Layout/DotPosition: - EnforcedStyle: leading - SupportedStyles: - - leading - - trailing - -Style/EmptyElse: - EnforcedStyle: both - SupportedStyles: - - empty - - nil - - both - -Layout/EmptyLineBetweenDefs: - AllowAdjacentOneLineDefs: false - -Layout/EmptyLinesAroundBlockBody: - EnforcedStyle: no_empty_lines - SupportedStyles: - - empty_lines - - no_empty_lines - -Layout/EmptyLinesAroundClassBody: - EnforcedStyle: no_empty_lines - SupportedStyles: - - empty_lines - - empty_lines_except_namespace - - no_empty_lines - -Layout/EmptyLinesAroundModuleBody: - EnforcedStyle: no_empty_lines - SupportedStyles: - - empty_lines - - empty_lines_except_namespace - - no_empty_lines - -Layout/ExtraSpacing: - AllowForAlignment: true - ForceEqualSignAlignment: false - -Naming/FileName: - Exclude: [] - ExpectMatchingDefinition: false - Regex: - IgnoreExecutableScripts: true - -Layout/FirstArgumentIndentation: - EnforcedStyle: consistent - SupportedStyles: - - consistent - - special_for_inner_method_call - - special_for_inner_method_call_in_parentheses - IndentationWidth: - -Style/For: - EnforcedStyle: each - SupportedStyles: - - for - - each - -Style/FormatString: - EnforcedStyle: format - SupportedStyles: - - format - - sprintf - - percent - -Style/FrozenStringLiteralComment: - Details: >- - Add `# frozen_string_literal: true` to the top of the file. Frozen string - literals will become the default in a future Ruby version, and we want to - make sure we're ready. - EnforcedStyle: always - SupportedStyles: - - always - - never - -Style/GlobalVars: - AllowedVariables: [] - -Style/HashSyntax: - EnforcedStyle: ruby19 - SupportedStyles: - - ruby19 - - hash_rockets - - no_mixed_keys - - ruby19_no_mixed_keys - UseHashRocketsWithSymbolValues: false - PreferHashRocketsForNonAlnumEndingSymbols: false - -Layout/IndentationConsistency: - EnforcedStyle: normal - SupportedStyles: - - normal - - rails - -Layout/IndentationWidth: - Width: 2 - -Layout/FirstArrayElementIndentation: - EnforcedStyle: consistent - SupportedStyles: - - special_inside_parentheses - - consistent - - align_brackets - IndentationWidth: - -Layout/AssignmentIndentation: - IndentationWidth: - -Layout/FirstHashElementIndentation: - EnforcedStyle: consistent - SupportedStyles: - - special_inside_parentheses - - consistent - - align_braces - IndentationWidth: - -Style/LambdaCall: - EnforcedStyle: call - SupportedStyles: - - call - - braces - -Style/Next: - EnforcedStyle: skip_modifier_ifs - MinBodyLength: 3 - SupportedStyles: - - skip_modifier_ifs - - always - -Style/NonNilCheck: - IncludeSemanticChanges: false - -Style/MethodCallWithArgsParentheses: - Enabled: true - IgnoreMacros: true - IgnoredMethods: - - require - - require_relative - - require_dependency - - yield - - raise - - puts - Exclude: - - Gemfile - -Style/MethodDefParentheses: - EnforcedStyle: require_parentheses - SupportedStyles: - - require_parentheses - - require_no_parentheses - - require_no_parentheses_except_multiline - -Naming/MethodName: - EnforcedStyle: snake_case - SupportedStyles: - - snake_case - - camelCase - -Layout/MultilineArrayBraceLayout: - EnforcedStyle: symmetrical - SupportedStyles: - - symmetrical - - new_line - - same_line - -Layout/MultilineHashBraceLayout: - EnforcedStyle: symmetrical - SupportedStyles: - - symmetrical - - new_line - - same_line - -Layout/MultilineMethodCallBraceLayout: - EnforcedStyle: symmetrical - SupportedStyles: - - symmetrical - - new_line - - same_line - -Layout/MultilineMethodCallIndentation: - EnforcedStyle: indented - SupportedStyles: - - aligned - - indented - - indented_relative_to_receiver - IndentationWidth: 2 - -Layout/MultilineMethodDefinitionBraceLayout: - EnforcedStyle: symmetrical - SupportedStyles: - - symmetrical - - new_line - - same_line - -Style/NumericLiteralPrefix: - EnforcedOctalStyle: zero_only - SupportedOctalStyles: - - zero_with_o - - zero_only - -Style/ParenthesesAroundCondition: - AllowSafeAssignment: true - -Style/PercentQLiterals: - EnforcedStyle: lower_case_q - SupportedStyles: - - lower_case_q - - upper_case_q - -Naming/PredicateName: - NamePrefix: - - is_ - ForbiddenPrefixes: - - is_ - AllowedMethods: - - is_a? - Exclude: - - 'spec/**/*' - -Style/PreferredHashMethods: - EnforcedStyle: short - SupportedStyles: - - short - - verbose - -Style/RaiseArgs: - EnforcedStyle: exploded - SupportedStyles: - - compact - - exploded - -Style/RedundantReturn: - AllowMultipleReturnValues: false - -Style/RegexpLiteral: - EnforcedStyle: mixed - SupportedStyles: - - slashes - - percent_r - - mixed - AllowInnerSlashes: false - -Style/SafeNavigation: - ConvertCodeThatCanStartToReturnNil: false - Enabled: true - -Lint/SafeNavigationChain: - Enabled: true - -Style/Semicolon: - AllowAsExpressionSeparator: false - -Style/SignalException: - EnforcedStyle: only_raise - SupportedStyles: - - only_raise - - only_fail - - semantic - -Style/SingleLineMethods: - AllowIfMethodIsEmpty: true - -Layout/SpaceBeforeFirstArg: - AllowForAlignment: true - -Style/SpecialGlobalVars: - EnforcedStyle: use_english_names - SupportedStyles: - - use_perl_names - - use_english_names - -Style/StabbyLambdaParentheses: - EnforcedStyle: require_parentheses - SupportedStyles: - - require_parentheses - - require_no_parentheses - -Style/StringLiteralsInInterpolation: - EnforcedStyle: single_quotes - SupportedStyles: - - single_quotes - - double_quotes - -Layout/SpaceAroundBlockParameters: - EnforcedStyleInsidePipes: no_space - SupportedStylesInsidePipes: - - space - - no_space - -Layout/SpaceAroundEqualsInParameterDefault: - EnforcedStyle: space - SupportedStyles: - - space - - no_space - -Layout/SpaceAroundOperators: - AllowForAlignment: true - -Layout/SpaceBeforeBlockBraces: - EnforcedStyle: space - EnforcedStyleForEmptyBraces: space - SupportedStyles: - - space - - no_space - -Layout/SpaceInsideBlockBraces: - EnforcedStyle: space - SupportedStyles: - - space - - no_space - EnforcedStyleForEmptyBraces: no_space - SpaceBeforeBlockParameters: true - -Layout/SpaceInsideHashLiteralBraces: - EnforcedStyle: space - EnforcedStyleForEmptyBraces: no_space - SupportedStyles: - - space - - no_space - - compact - -Layout/SpaceInsideStringInterpolation: - EnforcedStyle: no_space - SupportedStyles: - - space - - no_space - -Style/SymbolProc: - IgnoredMethods: - - respond_to - - define_method - -Style/TernaryParentheses: - EnforcedStyle: require_no_parentheses - SupportedStyles: - - require_parentheses - - require_no_parentheses - AllowSafeAssignment: true - -Layout/TrailingEmptyLines: - EnforcedStyle: final_newline - SupportedStyles: - - final_newline - - final_blank_line - -Style/TrivialAccessors: - ExactNameMatch: true - AllowPredicates: true - AllowDSLWriters: false - IgnoreClassMethods: false - AllowedMethods: - - to_ary - - to_a - - to_c - - to_enum - - to_h - - to_hash - - to_i - - to_int - - to_io - - to_open - - to_path - - to_proc - - to_r - - to_regexp - - to_str - - to_s - - to_sym - -Naming/VariableName: - EnforcedStyle: snake_case - SupportedStyles: - - snake_case - - camelCase - -Style/WhileUntilModifier: - Enabled: true - -Metrics/BlockNesting: - Max: 3 - -Layout/LineLength: - Max: 120 - AllowHeredoc: true - AllowURI: true - URISchemes: - - http - - https - IgnoreCopDirectives: false - IgnoredPatterns: - - '\A\s*(remote_)?test(_\w+)?\s.*(do|->)(\s|\Z)' - -Metrics/ParameterLists: - Max: 5 - CountKeywordArgs: false - -Layout/BlockAlignment: - EnforcedStyleAlignWith: either - SupportedStylesAlignWith: - - either - - start_of_block - - start_of_line - -Layout/EndAlignment: - EnforcedStyleAlignWith: variable - SupportedStylesAlignWith: - - keyword - - variable - - start_of_line - -Layout/DefEndAlignment: - EnforcedStyleAlignWith: start_of_line - SupportedStylesAlignWith: - - start_of_line - - def - -Lint/InheritException: - EnforcedStyle: runtime_error - SupportedStyles: - - runtime_error - - standard_error - -Lint/UnusedBlockArgument: - IgnoreEmptyBlocks: true - AllowUnusedKeywordArguments: false - -Lint/UnusedMethodArgument: - AllowUnusedKeywordArguments: false - IgnoreEmptyMethods: true - -Naming/AccessorMethodName: - Enabled: true - -Layout/ArrayAlignment: - Enabled: true - -Style/ArrayJoin: - Enabled: true - -Naming/AsciiIdentifiers: - Enabled: true - -Style/Attr: - Enabled: true - -Style/BeginBlock: - Enabled: true - -Style/BlockComments: - Enabled: true - -Layout/BlockEndNewline: - Enabled: true - -Style/CaseEquality: - Enabled: true - AllowOnConstant: true - -Style/CharacterLiteral: - Enabled: true - -Naming/ClassAndModuleCamelCase: - Enabled: true - -Style/ClassMethods: - Enabled: true - -Style/ClassVars: - Enabled: true - -Layout/ClosingParenthesisIndentation: - Enabled: true - -Style/ColonMethodCall: - Enabled: true - -Layout/CommentIndentation: - Enabled: true - -Naming/ConstantName: - Enabled: true - -Style/DateTime: - Enabled: true - -Style/DefWithParentheses: - Enabled: true - -Style/EachForSimpleLoop: - Enabled: true - -Style/EachWithObject: - Enabled: true - -Layout/ElseAlignment: - Enabled: true - -Style/EmptyCaseCondition: - Enabled: true - -Layout/EmptyLines: - Enabled: true - -Layout/EmptyLinesAroundAccessModifier: - Enabled: true - -Layout/EmptyLinesAroundMethodBody: - Enabled: true - -Style/EmptyLiteral: - Enabled: true - -Style/EndBlock: - Enabled: true - -Layout/EndOfLine: - Enabled: true - -Style/EvenOdd: - Enabled: true - -Layout/InitialIndentation: - Enabled: true - -Lint/FlipFlop: - Enabled: true - -Style/IfInsideElse: - Enabled: true - -Style/IfUnlessModifierOfIfUnless: - Enabled: true - -Style/IfWithSemicolon: - Enabled: true - -Style/IdenticalConditionalBranches: - Enabled: true - -Layout/IndentationStyle: - Enabled: true - -Style/InfiniteLoop: - Enabled: true - -Layout/LeadingCommentSpace: - Enabled: true - -Style/LineEndConcatenation: - Enabled: true - -Style/MethodCallWithoutArgsParentheses: - Enabled: true - -Style/MethodMissingSuper: - Enabled: true - -Style/MissingRespondToMissing: - Enabled: true - -Layout/MultilineBlockLayout: - Enabled: true - -Style/MultilineIfThen: - Enabled: true - -Style/MultilineMemoization: - Enabled: true - -Style/MultilineTernaryOperator: - Enabled: true - -Style/NegatedIf: - Enabled: true - -Style/NegatedWhile: - Enabled: true - -Style/NestedModifier: - Enabled: true - -Style/NestedParenthesizedCalls: - Enabled: true - -Style/NestedTernaryOperator: - Enabled: true - -Style/NilComparison: - Enabled: true - -Style/Not: - Enabled: true - -Style/OneLineConditional: - Enabled: true - -Naming/BinaryOperatorParameterName: - Enabled: true - -Style/OptionalArguments: - Enabled: true - -Style/ParallelAssignment: - Enabled: true - -Style/PerlBackrefs: - Enabled: true - -Style/Proc: - Enabled: true - -Style/RedundantBegin: - Enabled: true - -Style/RedundantException: - Enabled: true - -Style/RedundantFreeze: - Enabled: true - -Style/RedundantParentheses: - Enabled: true - -Style/RedundantSelf: - Enabled: true - -Style/RedundantSortBy: - Enabled: true - -Layout/RescueEnsureAlignment: - Enabled: true - -Style/RescueModifier: - Enabled: true - -Style/Sample: - Enabled: true - -Style/SelfAssignment: - Enabled: true - -Layout/SpaceAfterColon: - Enabled: true - -Layout/SpaceAfterComma: - Enabled: true - -Layout/SpaceAfterMethodName: - Enabled: true - -Layout/SpaceAfterNot: - Enabled: true - -Layout/SpaceAfterSemicolon: - Enabled: true - -Layout/SpaceBeforeComma: - Enabled: true - -Layout/SpaceBeforeComment: - Enabled: true - -Layout/SpaceBeforeSemicolon: - Enabled: true - -Layout/SpaceAroundKeyword: - Enabled: true - -Layout/SpaceInsideArrayPercentLiteral: - Enabled: true - -Layout/SpaceInsidePercentLiteralDelimiters: - Enabled: true - -Layout/SpaceInsideArrayLiteralBrackets: - Enabled: true - -Layout/SpaceInsideParens: - Enabled: true - -Layout/SpaceInsideRangeLiteral: - Enabled: true - -Style/SymbolLiteral: - Enabled: true - -Layout/TrailingWhitespace: - Enabled: true - -Style/UnlessElse: - Enabled: true - -Style/RedundantCapitalW: - Enabled: true - -Style/RedundantInterpolation: - Enabled: true - -Style/RedundantPercentQ: - Enabled: true - -Style/VariableInterpolation: - Enabled: true - -Style/WhenThen: - Enabled: true - -Style/WhileUntilDo: - Enabled: true - -Style/ZeroLengthPredicate: - Enabled: true - -Layout/HeredocIndentation: - Enabled: true - -Lint/AmbiguousOperator: - Enabled: true - -Lint/AmbiguousRegexpLiteral: - Enabled: true - -Lint/CircularArgumentReference: - Enabled: true - -Layout/ConditionPosition: - Enabled: true - -Lint/Debugger: - Enabled: true - -Lint/DeprecatedClassMethods: - Enabled: true - -Lint/DuplicateMethods: - Enabled: true - -Lint/DuplicateHashKey: - Enabled: true - -Lint/EachWithObjectArgument: - Enabled: true - -Lint/ElseLayout: - Enabled: true - -Lint/EmptyEnsure: - Enabled: true - -Lint/EmptyInterpolation: - Enabled: true - -Lint/EnsureReturn: - Enabled: true - -Lint/FloatOutOfRange: - Enabled: true - -Lint/FormatParameterMismatch: - Enabled: true - -Lint/SuppressedException: - AllowComments: true - -Lint/ImplicitStringConcatenation: - Description: Checks for adjacent string literals on the same line, which could - better be represented as a single string literal. - -Lint/IneffectiveAccessModifier: - Description: Checks for attempts to use `private` or `protected` to set the visibility - of a class method, which does not work. - -Lint/LiteralAsCondition: - Enabled: true - -Lint/LiteralInInterpolation: - Enabled: true - -Lint/Loop: - Description: Use Kernel#loop with break rather than begin/end/until or begin/end/while - for post-loop tests. - -Lint/NestedMethodDefinition: - Enabled: true - -Lint/NextWithoutAccumulator: - Description: Do not omit the accumulator when calling `next` in a `reduce`/`inject` - block. - -Lint/NonLocalExitFromIterator: - Enabled: true - -Lint/ParenthesesAsGroupedExpression: - Enabled: true - -Lint/PercentStringArray: - Enabled: true - -Lint/PercentSymbolArray: - Enabled: true - -Lint/RandOne: - Description: Checks for `rand(1)` calls. Such calls always return `0` and most - likely a mistake. - -Lint/RequireParentheses: - Enabled: true - -Lint/RescueException: - Enabled: true - -Lint/ShadowedException: - Enabled: true - -Lint/ShadowingOuterLocalVariable: - Enabled: true - -Lint/RedundantStringCoercion: - Enabled: true - -Lint/UnderscorePrefixedVariableName: - Enabled: true - -Lint/UnifiedInteger: - Enabled: true - -Lint/RedundantCopDisableDirective: - Enabled: true - -Lint/RedundantCopEnableDirective: - Enabled: true - -Lint/RedundantSplatExpansion: - Enabled: true - -Lint/UnreachableCode: - Enabled: true - -Lint/UselessAccessModifier: - ContextCreatingMethods: [] - -Lint/UselessAssignment: - Enabled: true - -Lint/UselessComparison: - Enabled: true - -Lint/UselessElseWithoutRescue: - Enabled: true - -Lint/UselessSetterCall: - Enabled: true - -Lint/Void: - Enabled: true - -Security/Eval: - Enabled: true - -Security/JSONLoad: - Enabled: true - -Security/Open: - Enabled: true - -Lint/BigDecimalNew: - Enabled: true - -Style/Strip: - Enabled: true - -Style/TrailingBodyOnClass: - Enabled: true - -Style/TrailingBodyOnModule: - Enabled: true - -Style/TrailingCommaInArrayLiteral: - EnforcedStyleForMultiline: comma - Enabled: true - -Style/TrailingCommaInHashLiteral: - EnforcedStyleForMultiline: comma - Enabled: true - -Layout/SpaceInsideReferenceBrackets: - EnforcedStyle: no_space - EnforcedStyleForEmptyBrackets: no_space - Enabled: true - -Style/ModuleFunction: - EnforcedStyle: extend_self - -Lint/OrderedMagicComments: - Enabled: true - -Lint/DeprecatedOpenSSLConstant: - Enabled: true diff --git a/gems/quilt_rails/.rubocop.yml b/gems/quilt_rails/.rubocop.yml index 06b4bf5d93..38c62684e6 100644 --- a/gems/quilt_rails/.rubocop.yml +++ b/gems/quilt_rails/.rubocop.yml @@ -1,8 +1,8 @@ # This file strictly follows the rules defined in the Ruby style guide: # http://shopify.github.io/ruby-style-guide/ # Before updating anything please sync-up with #ruby-style-guide on Slack. -inherit_from: - - http://shopify.github.io/ruby-style-guide/rubocop.yml +inherit_gem: + rubocop-shopify: rubocop.yml AllCops: TargetRubyVersion: 2.6.3 @@ -12,13 +12,12 @@ AllCops: - 'vendor/**/*' - '*.gemspec' -Rails: - Enabled: false - Metrics/PerceivedComplexity: Max: 6 + Exclude: + - bin/**/* -CyclomaticComplexity: +Metrics/CyclomaticComplexity: Max: 6 Exclude: - bin/**/* @@ -41,16 +40,6 @@ Metrics/BlockLength: - config/initializers/**/* - config/environments/**/* -Metrics/BlockNesting: - CountBlocks: false - Max: 1 - Exclude: - - lib/**/*.rake - - test/**/* - - bin/**/* - - Gemfile - - config/initializers/**/* - Metrics/ClassLength: CountComments: true Max: 100 @@ -64,7 +53,7 @@ Naming/MethodName: Style/ClassAndModuleChildren: Enabled: false -Metrics/LineLength: +Layout/LineLength: Max: 120 Exclude: - Guardfile @@ -72,10 +61,6 @@ Metrics/LineLength: - config/**/* - bin/**/* -Metrics/PerceivedComplexity: - Exclude: - - bin/**/* - Style/MethodCallWithArgsParentheses: Exclude: - bin/**/* diff --git a/gems/quilt_rails/Gemfile b/gems/quilt_rails/Gemfile index 26e543f52d..8d6d0ec368 100644 --- a/gems/quilt_rails/Gemfile +++ b/gems/quilt_rails/Gemfile @@ -3,6 +3,8 @@ source('https://rubygems.org') gemspec +gem('rubocop-shopify', require: false) + group :test do gem 'minitest' gem 'minitest-hooks' diff --git a/gems/quilt_rails/Gemfile.lock b/gems/quilt_rails/Gemfile.lock index 229ac96879..d2278fd043 100644 --- a/gems/quilt_rails/Gemfile.lock +++ b/gems/quilt_rails/Gemfile.lock @@ -66,9 +66,9 @@ GEM zeitwerk (~> 2.2, >= 2.2.2) addressable (2.7.0) public_suffix (>= 2.0.2, < 5.0) - ast (2.4.0) + ast (2.4.1) builder (3.2.4) - coderay (1.1.2) + coderay (1.1.3) concurrent-ruby (1.1.7) crass (1.0.6) erubi (1.9.0) @@ -76,7 +76,6 @@ GEM activesupport (>= 4.2.0) i18n (1.8.5) concurrent-ruby (~> 1.0) - jaro_winkler (1.5.3) loofah (2.6.0) crass (~> 1.0.2) nokogiri (>= 1.5.9) @@ -84,25 +83,23 @@ GEM mini_mime (>= 0.1.1) marcel (0.3.3) mimemagic (~> 0.3.2) - metaclass (0.0.4) - method_source (0.9.2) + method_source (1.0.0) mimemagic (0.3.5) mini_mime (1.0.2) mini_portile2 (2.4.0) - minitest (5.11.3) + minitest (5.14.1) minitest-hooks (1.5.0) minitest (> 5.3) - mocha (1.9.0) - metaclass (~> 0.0.1) + mocha (1.11.2) nio4r (2.5.2) nokogiri (1.10.10) mini_portile2 (~> 2.4.0) - parallel (1.17.0) - parser (2.6.3.0) - ast (~> 2.4.0) - pry (0.12.2) - coderay (~> 1.1.0) - method_source (~> 0.9.0) + parallel (1.19.2) + parser (2.7.1.4) + ast (~> 2.4.1) + pry (0.13.1) + coderay (~> 1.1) + method_source (~> 1.0) public_suffix (4.0.5) rack (2.2.3) rack-proxy (0.6.5) @@ -142,15 +139,23 @@ GEM thor (>= 0.20.3, < 2.0) rainbow (3.0.0) rake (13.0.1) - rubocop (0.74.0) - jaro_winkler (~> 1.5.1) + regexp_parser (1.7.1) + rexml (3.2.4) + rubocop (0.86.0) parallel (~> 1.10) - parser (>= 2.6) + parser (>= 2.7.0.1) rainbow (>= 2.2.2, < 4.0) + regexp_parser (>= 1.7) + rexml + rubocop-ast (>= 0.0.3, < 1.0) ruby-progressbar (~> 1.7) - unicode-display_width (>= 1.4.0, < 1.7) + unicode-display_width (>= 1.4.0, < 2.0) + rubocop-ast (0.3.0) + parser (>= 2.7.1.4) rubocop-git (0.1.3) rubocop (>= 0.24.1) + rubocop-shopify (1.0.4) + rubocop (>= 0.85, < 0.87) ruby-progressbar (1.10.1) sprockets (4.0.2) concurrent-ruby (~> 1.0) @@ -165,7 +170,7 @@ GEM thread_safe (0.3.6) tzinfo (1.2.7) thread_safe (~> 0.1) - unicode-display_width (1.6.0) + unicode-display_width (1.7.0) websocket-driver (0.7.3) websocket-extensions (>= 0.1.0) websocket-extensions (0.1.5) @@ -182,6 +187,7 @@ DEPENDENCIES quilt_rails! rubocop (~> 0.74) rubocop-git (~> 0.1.3) + rubocop-shopify sqlite3 BUNDLED WITH diff --git a/gems/quilt_rails/Rakefile b/gems/quilt_rails/Rakefile index 384d4d46f6..9724784e5a 100644 --- a/gems/quilt_rails/Rakefile +++ b/gems/quilt_rails/Rakefile @@ -24,3 +24,5 @@ Rake::TestTask.new do |t| t.name = 'test:unit' t.pattern = 'test/quilt_rails/**/*_test.rb' end + +task(default: %i(test)) diff --git a/gems/quilt_rails/app/controllers/quilt/performance_report_controller.rb b/gems/quilt_rails/app/controllers/quilt/performance_report_controller.rb index 4077336a22..97d8cc2751 100644 --- a/gems/quilt_rails/app/controllers/quilt/performance_report_controller.rb +++ b/gems/quilt_rails/app/controllers/quilt/performance_report_controller.rb @@ -8,9 +8,9 @@ class PerformanceReportController < ActionController::Base def create process_report - render json: { result: 'success' }, status: 200 + render(json: { result: 'success' }, status: 200) rescue ActionController::ParameterMissing => error - render json: { error: error.message, status: 422 } + render(json: { error: error.message, status: 422 }) end end end diff --git a/gems/quilt_rails/lib/generators/quilt/rails_setup/rails_setup_generator.rb b/gems/quilt_rails/lib/generators/quilt/rails_setup/rails_setup_generator.rb index 0c14246536..bf7cb95615 100644 --- a/gems/quilt_rails/lib/generators/quilt/rails_setup/rails_setup_generator.rb +++ b/gems/quilt_rails/lib/generators/quilt/rails_setup/rails_setup_generator.rb @@ -12,7 +12,7 @@ def create_procfile_entry if File.exist?(procfile_path) append_file(procfile_path, File.read(File.expand_path(find_in_source_paths(procfile_path)))) else - copy_file procfile_path + copy_file(procfile_path) end end @@ -20,12 +20,12 @@ def create_route_file routes_path = "config/routes.rb" if File.exist?(routes_path) - route "mount Quilt::Engine, at: '/'" + route("mount Quilt::Engine, at: '/'") else - copy_file "routes.rb", routes_path + copy_file("routes.rb", routes_path) end - say "Added Quilt engine mount" + say("Added Quilt engine mount") end end end diff --git a/gems/quilt_rails/lib/generators/quilt/react_setup/react_setup_generator.rb b/gems/quilt_rails/lib/generators/quilt/react_setup/react_setup_generator.rb index 8d9717fa45..3d034ef1b5 100644 --- a/gems/quilt_rails/lib/generators/quilt/react_setup/react_setup_generator.rb +++ b/gems/quilt_rails/lib/generators/quilt/react_setup/react_setup_generator.rb @@ -10,7 +10,7 @@ class ReactSetupGenerator < Rails::Generators::Base def install_js_dependencies return if options.skip_yarn? - say "Installing react and types dependencies" + say("Installing react and types dependencies") system("yarn add "\ "typescript@~3.8.0 "\ "react@~16.11.0 "\ diff --git a/gems/quilt_rails/lib/quilt_rails/configuration.rb b/gems/quilt_rails/lib/quilt_rails/configuration.rb index 2dc25378bb..11ae093ab9 100644 --- a/gems/quilt_rails/lib/quilt_rails/configuration.rb +++ b/gems/quilt_rails/lib/quilt_rails/configuration.rb @@ -19,7 +19,6 @@ def mount? end end - def self.configuration @configuration ||= Configuration.new end diff --git a/gems/quilt_rails/lib/quilt_rails/engine.rb b/gems/quilt_rails/lib/quilt_rails/engine.rb index 501498f1ce..06378bb64b 100644 --- a/gems/quilt_rails/lib/quilt_rails/engine.rb +++ b/gems/quilt_rails/lib/quilt_rails/engine.rb @@ -7,9 +7,11 @@ class Engine < ::Rails::Engine config.quilt = Quilt.configuration initializer(:mount_quilt, before: :add_builtin_route) do |app| - app.routes.append do - mount(Quilt::Engine, at: '/') unless has_named_route?(:quilt) - end if config.quilt.mount? + if config.quilt.mount? + app.routes.append do + mount(Quilt::Engine, at: '/') unless has_named_route?(:quilt) + end + end end end end diff --git a/gems/quilt_rails/lib/quilt_rails/header_csrf_strategy.rb b/gems/quilt_rails/lib/quilt_rails/header_csrf_strategy.rb index d26145f158..a38f7eedec 100644 --- a/gems/quilt_rails/lib/quilt_rails/header_csrf_strategy.rb +++ b/gems/quilt_rails/lib/quilt_rails/header_csrf_strategy.rb @@ -25,9 +25,10 @@ def fallback_handler class NoSameSiteHeaderError < StandardError def initialize - # rubocop:disable LineLength - super "CSRF verification failed. This request is missing the `x-shopify-react-xhr` header, or it does not have the expected value." - # rubocop:enable LineLength + super(<<~MSG.squish) + CSRF verification failed. This request is missing the + `x-shopify-react-xhr` header, or it does not have the expected value. + MSG end end end diff --git a/gems/quilt_rails/lib/quilt_rails/performance/report.rb b/gems/quilt_rails/lib/quilt_rails/performance/report.rb index 9eb1f1315e..f7bbbe77aa 100644 --- a/gems/quilt_rails/lib/quilt_rails/performance/report.rb +++ b/gems/quilt_rails/lib/quilt_rails/performance/report.rb @@ -7,25 +7,39 @@ class Report attr_accessor :navigations attr_accessor :connection - def self.from_params(params) - params.transform_keys! { |key| key.underscore.to_sym } - params[:connection] = { effectiveType: 'unknown' } if params[:connection].blank? + class << self + def from_params(params) + params.transform_keys! { |key| key.underscore.to_sym } + params[:connection] = { effectiveType: 'unknown' } if params[:connection].blank? - connection = Connection.from_params(params[:connection]) + connection = Connection.from_params(params[:connection]) - Report.new( - connection: connection, - navigations: (params[:navigations] || []).map do |navigation| + Report.new( + connection: connection, + navigations: build_navigations(params[:navigations], connection: connection), + events: build_events(params[:events], connection: connection), + ) + end + + private + + def build_navigations(navigations_params, connection:) + navigations_params ||= [] + navigations_params.map do |navigation| navigation = Navigation.from_params(navigation) navigation.connection = connection navigation - end, - events: (params[:events] || []).map do |event| + end + end + + def build_events(events_params, connection:) + events_params ||= [] + events_params.map do |event| event = Event.from_params(event) event.connection = connection event - end, - ) + end + end end def initialize(events:, navigations:, connection:) diff --git a/gems/quilt_rails/lib/quilt_rails/react_renderable.rb b/gems/quilt_rails/lib/quilt_rails/react_renderable.rb index 2b250e2934..5087119685 100644 --- a/gems/quilt_rails/lib/quilt_rails/react_renderable.rb +++ b/gems/quilt_rails/lib/quilt_rails/react_renderable.rb @@ -52,17 +52,19 @@ def proxy(headers, data) class ReactServerNoResponseError < StandardError def initialize(url) - # rubocop:disable LineLength - super "Errno::ECONNREFUSED: Waiting for React server to boot up. If this error persists verify that @shopify/react-server is configured on #{url}" - # rubocop:enable LineLength + super(<<~MSG.squish) + Errno::ECONNREFUSED: Waiting for React server to boot up. + If this error persists verify that @shopify/react-server is configured on #{url}" + MSG end end class DoNotIntegrationTestError < StandardError def initialize - # rubocop:disable LineLength - super "Do not try to use Rails integration tests on your quilt_rails app. Instead use Jest and @shopify/react-testing to test your React application directly." - # rubocop:enable LineLength + super(<<~MSG.squish) + Do not try to use Rails integration tests on your quilt_rails app. + Instead use Jest and @shopify/react-testing to test your React application directly." + MSG end end end