Skip to content
This repository

NeuterFilter works with additional_dependencies #46

Open
wants to merge 7 commits into from

2 participants

Adam Hawkins Peter Wagenet
Adam Hawkins

All tests pass at the moment. I'm currently looking for a way to refactor the relationship between the NeuterFilter, NeuterWrapper and NeuterBatch because the code feels very odd. It's especially odd since now there additional responsibilities to implement additional_dependencies.

Peter Wagenet
Collaborator

Does additional_dependencies not work as it stands?

Adam Hawkins

@wagenet it did IIRC if you explicitly declared all the required files. This PR takes that step out and make require detection automatic.

Adam Hawkins

@wagenet @wycats ping, this should really be merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
68  lib/rake-pipeline-web-filters/neuter_filter.rb
... ...
@@ -1,9 +1,8 @@
1 1
 module Rake::Pipeline::Web::Filters
2 2
 
3 3
   class NeuterBatch
4  
-    def initialize(config, known_files)
  4
+    def initialize(config)
5 5
       @config = config || {}
6  
-      @known_files = known_files
7 6
       @required = []
8 7
     end
9 8
 
@@ -15,9 +14,6 @@ def file_wrapper(klass, *args)
15 14
     end
16 15
 
17 16
     def required(req)
18  
-      unless @known_files.include?(req)
19  
-        warn "Included '#{req}', which is not listed in :additional_dependencies. The pipeline may not invalidate properly."
20  
-      end
21 17
       @required << req
22 18
     end
23 19
 
@@ -25,12 +21,8 @@ def required?(req)
25 21
       @required.include?(req)
26 22
     end
27 23
 
28  
-    def strip_requires(source)
29  
-      requires = []
30  
-      regexp = @config[:require_regexp] || %r{^\s*require\(['"]([^'"]*)['"]\);?\s*}
31  
-      # FIXME: This $1 may not be reliable with other regexps
32  
-      source.gsub!(regexp){ requires << $1; '' }
33  
-      requires
  24
+    def regexp
  25
+      @config[:require_regexp] || %r{^\s*require\(['"]([^'"]*)['"]\);?\s*}
34 26
     end
35 27
 
36 28
     def transform_path(path, input)
@@ -49,24 +41,38 @@ def filename_comment(input)
49 41
   module NeuterWrapper
50 42
     def batch(batch)
51 43
       @batch = batch
52  
-      @batch.required(fullpath)
53 44
     end
54 45
 
55  
-    def read
56  
-      source = super
  46
+    def neuter
  47
+      return if required?
57 48
 
58  
-      required_files = @batch.strip_requires(source).map do |req|
59  
-        req_path = @batch.transform_path(req, self)
60  
-        if req_path && !@batch.required?(File.expand_path(req_path, root))
61  
-          @batch.file_wrapper(self.class, root, req_path, encoding).read
62  
-        else
63  
-          nil
64  
-        end
65  
-      end.compact
  49
+      @batch.required fullpath
  50
+
  51
+      files_to_inject = dependencies.reject(&:required?)
  52
+      dependent_content = files_to_inject.map(&:neuter).compact
  53
+
  54
+      this_file = @batch.filename_comment(self) + @batch.closure_wrap(stripped_source)
66 55
 
67  
-      file = @batch.filename_comment(self) + @batch.closure_wrap(source)
  56
+      [dependent_content, this_file].reject(&:empty?).join("\n\n")
  57
+    end
  58
+
  59
+    def required?
  60
+      @batch.required? fullpath
  61
+    end
68 62
 
69  
-      (required_files << file).join("\n\n")
  63
+    def requires
  64
+      read.scan(@batch.regexp).flatten
  65
+    end
  66
+
  67
+    def dependencies
  68
+      requires.map do |req|
  69
+        req_path = @batch.transform_path(req, self)
  70
+        @batch.file_wrapper(self.class, root, req_path, encoding)
  71
+      end
  72
+    end
  73
+
  74
+    def stripped_source
  75
+      read.gsub @batch.regexp, ''
70 76
     end
71 77
   end
72 78
 
@@ -95,16 +101,20 @@ def initialize(string=nil, config={}, &block)
95 101
 
96 102
     def generate_output(inputs, output)
97 103
       inputs.each do |input|
98  
-        known_files = [input.fullpath] + additional_dependencies(input)
99  
-        batch = NeuterBatch.new(@config, known_files)
  104
+        batch = NeuterBatch.new @config
100 105
         file = batch.file_wrapper(file_wrapper_class, input.root, input.path, input.encoding)
101  
-        output.write file.read
  106
+        output.write file.neuter
102 107
       end
103 108
     end
104 109
 
105 110
     def additional_dependencies(input)
106  
-      method = @config[:additional_dependencies]
107  
-      method ? method.call(input).map{|p| File.expand_path(p, input.root) } : []
  111
+      dependent_files(input).map(&:fullpath)
  112
+    end
  113
+
  114
+    def dependent_files(input)
  115
+      batch = NeuterBatch.new @config
  116
+      wrapper = batch.file_wrapper(file_wrapper_class, input.root, input.path, input.encoding)
  117
+      wrapper.dependencies
108 118
     end
109 119
   end
110 120
 end
46  spec/neuter_filter_spec.rb
@@ -14,7 +14,6 @@ def make_data(name, data)
14 14
 
15 15
   def make_filter(input_files, *args)
16 16
     opts = args.last.is_a?(Hash) ? args.pop : {}
17  
-    opts[:additional_dependencies] ||= proc{|input| %w(b c) }
18 17
     args.push(opts)
19 18
 
20 19
     filter = Rake::Pipeline::Web::Filters::NeuterFilter.new(*args)
@@ -83,7 +82,7 @@ def capture(*streams)
83 82
       ["lib/a", "require('lib/b');\nA"],
84 83
       ["lib/b", "require('lib/c');\nB"],
85 84
       ["lib/c", "C"]
86  
-    ], :additional_dependencies => proc{ %w(lib/b lib/c) })
  85
+    ])
87 86
 
88 87
     output_file.body.should == "C\n\nB\n\nA"
89 88
   end
@@ -140,8 +139,7 @@ def capture(*streams)
140 139
           ["lib/a.js", "require('b');\nA"],
141 140
           ["lib/b.js", "require('c');\nB"],
142 141
           ["lib/c.js", "C"]
143  
-        ], :path_transform => proc{|path| "lib/#{path}.js" },
144  
-           :additional_dependencies => proc{ %w(lib/b.js lib/c.js) })
  142
+        ], :path_transform => proc{|path| "lib/#{path}.js" })
145 143
 
146 144
         output_file.body.should == "C\n\nB\n\nA"
147 145
       end
@@ -175,29 +173,29 @@ def capture(*streams)
175 173
     end
176 174
 
177 175
     describe "additional_dependencies" do
178  
-      it "warns if required file is not contained" do
179  
-        output = capture(:stderr) do
180  
-          make_filter_with_inputs([
181  
-            ["d", "require('e');\nD"],
182  
-            ["e", "require('f');\nE"],
183  
-            ["f", "F"]
184  
-          ])
185  
-        end
186  
-
187  
-        output.should include("Included '/path/to/input/e', which is not listed in :additional_dependencies. The pipeline may not invalidate properly.")
188  
-        output.should include("Included '/path/to/input/f', which is not listed in :additional_dependencies. The pipeline may not invalidate properly.")
  176
+      let!(:main_input_file) { make_input("a", %Q{require("b");}) }
  177
+      let!(:required_input_file) { make_input("b", "foo") }
  178
+
  179
+      it "determines them from require statments in the file" do
  180
+        filter = Rake::Pipeline::Web::Filters::NeuterFilter.new
  181
+        filter.file_wrapper_class = MemoryFileWrapper
  182
+        filter.input_files = []
  183
+        filter.output_root = "/path/to/output"
  184
+        filter.rake_application = Rake::Application.new
  185
+
  186
+        dependencies = filter.additional_dependencies(main_input_file)
  187
+        dependencies.should include(required_input_file.fullpath)
189 188
       end
190 189
 
191  
-      it "does not warn if full paths are provided" do
192  
-        output = capture(:stderr) do
193  
-          make_filter_with_inputs([
194  
-            ["d", "require('e');\nD"],
195  
-            ["e", "require('f');\nE"],
196  
-            ["f", "F"]
197  
-          ], :additional_dependencies => proc{ %w(/path/to/input/e /path/to/input/f) })
198  
-        end
  190
+      it "determines there are no dependencies" do
  191
+        filter = Rake::Pipeline::Web::Filters::NeuterFilter.new
  192
+        filter.file_wrapper_class = MemoryFileWrapper
  193
+        filter.output_root = "/path/to/output"
  194
+        filter.rake_application = Rake::Application.new
  195
+        filter.input_files = []
199 196
 
200  
-        output.should == ""
  197
+        dependencies = filter.additional_dependencies(required_input_file)
  198
+        dependencies.should == []
201 199
       end
202 200
     end
203 201
   end
1  spec/sass_filter_spec.rb
@@ -143,5 +143,4 @@ def write_input_file(filename, contents='', root=tmp)
143 143
       tasks.each(&:invoke)
144 144
     end
145 145
   end
146  
-
147 146
 end
Commit_comment_tip

Tip: You can add notes to lines in a file. Hover to the left of a line to make a note

Something went wrong with that request. Please try again.