Sprockets 4 Compatibility #65

schneems · 2016-06-23T16:07:09Z

I'm able to get a really simple test app working with this gem with these changes, but so far haven't gotten the tests to pass.

bolandrm · 2016-06-23T16:20:10Z

cool, i may be able to help with the tests if needed. LMK

bolandrm · 2016-06-23T16:24:48Z

it looks like the issue might be with switching to env.register_transformer. it looks like the failed tests are trying to use the regular sass instead of sassc.

be careful that your example app is using sassc - i usually double check by intentionally creating a syntax error and seeing that it is SassC::SyntaxError instead of Sass::SyntaxError

schneems · 2016-06-25T15:31:36Z

In my example app I added a syntax error and it looks like it's coming from sassc

$ rm -rf tmp/cache/; rm -rf public/assets/; time RAILS_ENV=production rake assets:precompile
trash: tmp/cache/: path does not exist
trash: public/assets/: path does not exist
rake aborted!
SassC::SyntaxError: Error: Invalid CSS after "...ry-color: #333;": expected 1 selector or at-rule, was "$$$$LKJASLDKJFOISDJ"
        on line 2 of app/assets/stylesheets/foo.scss
>> $primary-color: #333;
   ---------------------^
/Users/richardschneeman/Documents/projects/sassc-rails-app/app/assets/config/manifest.js:3
/Users/richardschneeman/.gem/ruby/2.4.0/gems/sassc-1.9.0/lib/sassc/engine.rb:43:in `render'
/Users/richardschneeman/Documents/projects/sassc-rails/lib/sassc/rails/template.rb:34:in `block in call'
/Users/richardschneeman/Documents/projects/sprockets/lib/sprockets/utils.rb:137:in `module_include'
/Users/richardschneeman/Documents/projects/sassc-rails/lib/sassc/rails/template.rb:33:in `call'
/Users/richardschneeman/Documents/projects/sprockets/lib/sprockets/processor_utils.rb:84:in `call_processor'
/Users/richardschneeman/Documents/projects/sprockets/lib/sprockets/processor_utils.rb:66:in `block in call_processors'
/Users/richardschneeman/Documents/projects/sprockets/lib/sprockets/processor_utils.rb:65:in `reverse_each'
/Users/richardschneeman/Documents/projects/sprockets/lib/sprockets/processor_utils.rb:65:in `call_processors'
/Users/richardschneeman/Documents/projects/sprockets/lib/sprockets/processor_utils.rb:22:in `block in <class:CompositeProcessor>'
/Users/richardschneeman/Documents/projects/sprockets/lib/sprockets/processor_utils.rb:33:in `call'
/Users/richardschneeman/Documents/projects/sprockets/lib/sprockets/processor_utils.rb:84:in `call_processor'
/Users/richardschneeman/Documents/projects/sprockets/lib/sprockets/processor_utils.rb:66:in `block in call_processors'
/Users/richardschneeman/Documents/projects/sprockets/lib/sprockets/processor_utils.rb:65:in `reverse_each'
/Users/richardschneeman/Documents/projects/sprockets/lib/sprockets/processor_utils.rb:65:in `call_processors'

That was a good debugging idea. I'll have to dig into these test failures a bit more.

schneems · 2016-06-25T16:16:50Z

I did a bundle open added some debugging and ran tests.

It looks like no transformer is running, not the sassc-rails one nor the built in Sprockets one on these 9 failing tests.

In the loader right after we set processors I added

puts "======"
puts unloaded.compressed_path.inspect
puts processors.inspect

When I run sprockets tests I get results like this

======
"test/fixtures/sass/paths.scss?type=text/css&pipeline=self"
[#<Proc:0x007f94233c1248@/Users/richardschneeman/Documents/projects/sprockets/test/sprockets_test.rb:84>, #<struct Sprockets::ProcessorUtils::CompositeProcessor processor_strategy=#<Proc:0x007f9422070b08@/Users/richardschneeman/Documents/projects/sprockets/lib/sprockets/processor_utils.rb:22 (lambda)>, param=[#<Sprockets::Preprocessors::DefaultSourceMap:0x007f9423459188>, #<Sprockets::DirectiveProcessor:0x007f9423482df8 @header_pattern=/\A(?:(?m:\s*)(?:(?:\/\/.*\n?)+|(?:\/\*(?m:.*?)\*\/)))+/>, Sprockets::ScssProcessor], processors=[#<Sprockets::Preprocessors::DefaultSourceMap:0x007f9423459188>, #<Sprockets::DirectiveProcessor:0x007f9423482df8 @header_pattern=/\A(?:(?m:\s*)(?:(?:\/\/.*\n?)+|(?:\/\*(?m:.*?)\*\/)))+/>, Sprockets::ScssProcessor]>, #<Sprockets::DirectiveProcessor:0x007f9422ac6eb8 @header_pattern=/\A(?:(?m:\s*)(?:(?:\/\/.*\n?)+|(?:\/\*(?m:.*?)\*\/)))+/>, Sprockets::FileReader]

You can see where the Sprockets::ScssProcessor is being used.

However when I do the same with the scss-rails test suite I get results like this:

======
"app/assets/stylesheets/syntax_error.scss?type=text/scss"
[#<Sprockets::DirectiveProcessor:0x007fa1dac3e410 @header_pattern=/\A(?:(?m:\s*)(?:(?:\/\/.*\n?)+|(?:\/\*(?m:.*?)\*\/)))+/>, Sprockets::FileReader]

I think the difference is that Sprockets 4 will actually render your .scss file. Not the difference in the type= between the two one is text/css which works correctly the other is text/scss which isn't triggering the transformers.

So

new_css_output = render_asset("glob_test.scss")

In the tests should be

new_css_output = render_asset("glob_test.css")

bolandrm · 2016-06-25T16:20:41Z

Ah, that makes sense. Is that extension change backwards compatible?

Sprockets 3 supports both `register_transformer` and `register_engine` unfortunately they don't play well with one another i.e. if a "transformer" is registered using `register_engine` it behaves slightly differently than when registered via `register_transformer`. This cause problems because the default engines in `lib/sprockets.rb` that are registered all use `register_engine` so if we want to over-write the existing sass engine we have to do both calls when possible.

schneems · 2016-06-27T19:04:08Z

Got all tests green. Did have to use both register_transformer and register_engine to support Sprockets 3. Details of why are in the commit.

schneems · 2016-07-05T19:04:36Z

Anything else to do here? Any questions?

Frustratingly you may need to use both `register_engine` to get the behavior your want in Sprockets 3 even if you're using the new API (i.e. `register_transformer` etc.). I think this is because the different APIs have a different backend but I haven't looked into it enough to know exactly why they diverged. Here's a PR that shows the problem: sass/sassc-rails#65 If you only call `register_transformer` then the Sprockets 3 tests will fail. I think this is because the processor built into `lib/sprockets.rb` and registered via `register_engine` still gets executed. By registering the sassc processor again with `register_engine` we're writing over the built in processor to ensure ours takes priority. I'm not 100% on this cause but it seems reasonable. This PR introduces a way to silence the deprecation for `register_engine` so that libraries that still need the `register_engine` API to function correctly (such as sassc-rails) can operate without deprecations. This is an opt-in flag, we assume if you use the flag that you're aware of the new API and have updated your library appropriately. After this PR I'm going to change the docs to show how to use both `register_transformer` and `register_engine` at the same time.

samjewell · 2016-11-07T14:05:42Z

lib/sassc/rails/railtie.rb

-        env.register_engine '.scss', SassC::Rails::ScssTemplate
+        if env.respond_to?(:register_transformer)
+          env.register_transformer 'text/sass', 'text/css', SassC::Rails::SassTemplate.new #->() { puts "yoyoyoy" }
+          env.register_transformer 'text/scss', 'text/css', SassC::Rails::ScssTemplate.new #->() { puts "yoyoyoy" }


Did you mean to leave the puts "yoyoyoy" in at the end of these lines?

No, can you send me a PR to get rid of them?

Yeah I'll have a go tonight (UK time) 👍

jankeesvw · 2017-06-28T07:31:36Z

I just added issue #93, I think it's related.

fly938

1

schneems added 2 commits June 23, 2016 11:04

Default $ rake command to run tests

1a58a4e

Sprockets 4 Compatibility

775fbbd

I'm able to get a really simple test app working with this gem with these changes, but so far haven't gotten the tests to pass.

schneems mentioned this pull request Jun 25, 2016

Raise on accidental intermediate file format use rails/sprockets#327

Closed

Fix Sprockets 4 tests

c62a5ea

schneems force-pushed the schneems/sprockets4 branch from b0bb332 to c62a5ea Compare June 27, 2016 15:14

This was referenced Jun 27, 2016

Sassc/Scssc processors do not update imported files rails/sprockets#330

Closed

@import not working with sassc/scssc processor for gem libraries rails/sprockets#329

Open

bolandrm merged commit a7e1b5f into sass:master Jul 7, 2016

schneems mentioned this pull request Jul 20, 2016

Allow for register_engine without deprecation rails/sprockets#345

Merged

samjewell reviewed Nov 7, 2016

View reviewed changes

fly938 reviewed Dec 14, 2021

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Sprockets 4 Compatibility #65

Sprockets 4 Compatibility #65

schneems commented Jun 23, 2016

bolandrm commented Jun 23, 2016

bolandrm commented Jun 23, 2016

schneems commented Jun 25, 2016

schneems commented Jun 25, 2016

bolandrm commented Jun 25, 2016

schneems commented Jun 27, 2016

schneems commented Jul 5, 2016

samjewell Nov 7, 2016

schneems Nov 7, 2016

samjewell Nov 7, 2016

jankeesvw commented Jun 28, 2017

fly938 left a comment

Sprockets 4 Compatibility #65

Sprockets 4 Compatibility #65

Conversation

schneems commented Jun 23, 2016

bolandrm commented Jun 23, 2016

bolandrm commented Jun 23, 2016

schneems commented Jun 25, 2016

schneems commented Jun 25, 2016

bolandrm commented Jun 25, 2016

schneems commented Jun 27, 2016

schneems commented Jul 5, 2016

samjewell Nov 7, 2016

Choose a reason for hiding this comment

schneems Nov 7, 2016

Choose a reason for hiding this comment

samjewell Nov 7, 2016

Choose a reason for hiding this comment

jankeesvw commented Jun 28, 2017

fly938 left a comment

Choose a reason for hiding this comment