Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

"Add deprecations in v3.x" for passing Regexp to sprocket's valid_asset_uri? (string) method #117

Closed
jasnow opened this issue Aug 26, 2015 · 29 comments

Comments

@jasnow
Copy link

jasnow commented Aug 26, 2015

WIP: Passing Regexp to sprocket's valid_asset_uri? (string) method

  • MESSAGE: in valid_asset_uri?': undefined methodstart_with?' for #Regexp:0xb93ad04 (NoMethodError)
  • GEMS INVOLVED: sprockets (4.0.0)
    • gem 'sprockets', git: 'git://github.com/rails/sprockets.git'
    • gem 'sprockets-rails', '3.0.0.beta2'
  • AT: sprockets-a61550db7184/lib/sprockets/uri_utils.rb:75
  • STACK TRACE: https://gist.github.com/jasnow/2de57bd7f7212c10c8df

Unclear where the regexp input is inserted.
Questions or suggestions are welcome.

@schneems
Copy link
Member

This fixes it but tests are still red: #111

@jasnow
Copy link
Author

jasnow commented Aug 26, 2015

Thanks @schneems for the update. I will watch for it to be merged then I will close this as a dup.

@schneems
Copy link
Member

Sorry that was the wrong thing. This should already be in master a61550d

Try to update to point at that commit or later, it should be working. However absolute paths in the cache are still broken in master right now.

@jasnow
Copy link
Author

jasnow commented Aug 26, 2015

FYI: Still seeing the error message and here is the lines inside Gemfile.lock file (after upgrading):

  • remote: git://github.com/rails/sprockets.git
  • revision: fce29dc
  • MESSAGE: .rvm/gems/[email protected]/bundler/gems/sprockets-fce29dc1d794/lib/sprockets/uri_utils.rb:75:in valid_asset_uri?': undefined methodstart_with?' for #Regexp:0xab307b8 (NoMethodError)

@schneems
Copy link
Member

Can you give me a full backtrace? Did this work with sprockets 3.2.0?

@jasnow
Copy link
Author

jasnow commented Aug 26, 2015

Works in the Rails 4.2 version of repo:

  • sprockets (3.3.3)
  • sprockets-rails (2.3.2)

The above issue is with the Rails 5.0 version of the repo.

Here is the full backtrace (rake 2>&1 output): https://gist.github.com/jasnow/40abdf872082dadf8151

@schneems
Copy link
Member

Okay, so that's a different issue than the one I linked to earlier. Thanks for the help. Any chance you could give me a small project that reproduces the error, i'll be happy to dig in more.

@jasnow
Copy link
Author

jasnow commented Aug 26, 2015

Tried sprockets 3.0.0.beta2 version, edge sprockets, and rspec-rails 3.1.0 with Rails 4.2 version of repo and still works.

Therefore I will wait until rspec and devise ports to Rails 5. If you wish, I can close this for now and reopen later.

@schneems
Copy link
Member

Seems fine. Thanks for the report

@jasnow
Copy link
Author

jasnow commented Aug 26, 2015

Here is the specific problem regexp: /bootstrap/glyphicons-halflings-regular.(?:eot|svg|ttf|woff2?)$/

@jasnow
Copy link
Author

jasnow commented Aug 26, 2015

Also found this: method_missing in the previous gist:

.rvm/gems/[email protected]/bundler/gems/rails-2ceb16e539d1/railties/lib/rails/railtie.rb:194:in `method_missing'

@jasnow
Copy link
Author

jasnow commented Aug 26, 2015

Found where problem regexp is coming from:

  • Line 8 in gems/bootstrap-sass-3.3.5.1/lib/bootstrap-sass/engine.rb
    • app.config.assets.precompile << %r(bootstrap/glyphicons-halflings-regular.?:eot|svg|ttf|woff2?)$)

@schneems
Copy link
Member

I don't think this is a bug in sprockets, but rather a bug in sprockets-rails. Sprockets doesn't know anything about app.config.assets.precompile that's sprockets-rails it should expand that for us.

When run with sprockets-rails 2.3.2 we see that regex properly expanded

"/Users/richardschneeman/Documents/projects/codetriage/app/assets/stylesheets/application.css"
"/Users/richardschneeman/Documents/projects/codetriage/app/assets/stylesheets/application.css"
"/Users/richardschneeman/Documents/projects/codetriage/app/assets/stylesheets/bootstrap-responsive.min.css"
"/Users/richardschneeman/Documents/projects/codetriage/app/assets/stylesheets/bootstrap.min.css.erb"
"/Users/richardschneeman/Documents/projects/codetriage/app/assets/stylesheets/memory_view.css.scss"
"/Users/richardschneeman/Documents/projects/codetriage/app/assets/stylesheets/pagination.css"
"/Users/richardschneeman/Documents/projects/codetriage/app/assets/stylesheets/screen.scss"
"glyphicons-halflings.png"
"glyphicons-halflings-white.png"
"glyphicons-halflings.png"
"glyphicons-halflings.png"
"glyphicons-halflings.png"
"glyphicons-halflings.png"
"glyphicons-halflings-white.png"
"glyphicons-halflings-white.png"
"glyphicons-halflings-white.png"
"glyphicons-halflings-white.png"

However in sprockets-rails version 3.0.0.beta2 it is not expanded:

"manifest.js"
/bootstrap\/glyphicons-halflings-regular\.(?:eot|svg|ttf|woff2?)$/

@schneems
Copy link
Member

I'm closing this in favor of rails/sprockets-rails#269 it doesn't look like regexs are supported anymore

@jasnow
Copy link
Author

jasnow commented Aug 27, 2015

OK

@schneems
Copy link
Member

Thanks for the report.

@jasnow
Copy link
Author

jasnow commented Aug 27, 2015

Wish Github let use transfer an issue from one repo to another.

@schneems
Copy link
Member

They're linked (they can see I linked this issue from that one). You can click "subscribe" on the new issue to get updates.

@schneems schneems reopened this Aug 28, 2015
@schneems
Copy link
Member

Opening since it looks like I need to add deprecations in v3.x

@jasnow jasnow changed the title WIP: Passing Regexp to sprocket's valid_asset_uri? (string) method WIP: "Add deprecations in v3.x" for passing Regexp to sprocket's valid_asset_uri? (string) method Nov 13, 2015
@jasnow jasnow changed the title WIP: "Add deprecations in v3.x" for passing Regexp to sprocket's valid_asset_uri? (string) method "Add deprecations in v3.x" for passing Regexp to sprocket's valid_asset_uri? (string) method Nov 13, 2015
@jasnow
Copy link
Author

jasnow commented Dec 6, 2015

@schneems - Do you have examples of similar work that someone could look at to do this issue "add deprecations in v3.x"?

@schneems
Copy link
Member

schneems commented Dec 8, 2015

There's no good deprecation framework or pattern here. Sprockets was bad at publically letting people know that things were deprecated. Technically it is already deprecated, but there is no notice. Everything is in the legacy.rb file

What I think we want to do is add something like puts "Method <foo> is deprecated". We would want to wait until a beta of sprockets 4 is available and then release the deprecation in a minor version of sprockets 3 so anyone running that minor version.

I think that's what we want to do.

@jasnow
Copy link
Author

jasnow commented Mar 15, 2016

@schneems - Should this still be open?

@schneems
Copy link
Member

I'm starting to look into deprecations now. I have a branch for deprecations #264. We can merge other deprecations into that branch like #265 and when Sprockets 4 is out we can cut a deprecation release of Sprockets 3.

@jasnow
Copy link
Author

jasnow commented Apr 14, 2016

@schneems - Thanks for responding.

@jasnow
Copy link
Author

jasnow commented Apr 27, 2016

@schneems - Are ya'll finished with the above deprecations?

@schneems
Copy link
Member

Haven't really even started. There's definitely more things we can/should deprecated.

@jasnow
Copy link
Author

jasnow commented Apr 27, 2016

ok

@jasnow
Copy link
Author

jasnow commented May 7, 2016

@schneems - I always knew I was working with a hero - now you have be recognized. Congratulations!

@schneems
Copy link
Member

Is this a thing we still need to do? I now have a deprecation scheme in place in master and we are emitting deprecations. Does someone want to work on this? Can you give me a PR?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants