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

Kinda support manifest.compile with simple path #12

Merged
merged 3 commits into from Mar 24, 2015
Merged

Kinda support manifest.compile with simple path #12

merged 3 commits into from Mar 24, 2015

Conversation

josh
Copy link
Contributor

@josh josh commented Mar 24, 2015

Theres been a few issues around Manifest#compile targeting assets using transformers.

The issue has to do with how 2.x and 3.x implement manifest.compile. To support arbitrary procs and fnmatch syntax, sprockets has to iterate over every darn file in the load path to check it against these filters. Its terribly slow. It also works on full path names rather than logical path names. So manifest.compile "application.js" isn't the same as find_asset("application.js"), even though it mostly acts the same. The other times causes these reported bugs.

On 4.x, I removed all the compile magic and made manifest.compile "application.js" just do find_asset("application.js"). Its much simpler and faster but means you can't do this fnmatch stuff anymore.

For 3.x I was just leaving things how they worked on 2.x cause I couldn't really figure out how to still support the 2.x behavior without it. But, I think we can maybe detect if a path passed to compile is "simple" and doesn't use any fnmatch magic and we can just do a direct lookup. I feel like theres some other gotcha to this, but I think it might fix most the causing issues people are actually hitting.

Again, this hacks don't need to stick around in 4.x, this code path is basically deleted. This test case was actually cherry picked from the 4.x branch cause it already works there.

/cc @tricknotes @praxxis @rafaelfranca @lucasmazza

@josh josh added this to the 3.0 milestone Mar 24, 2015
@rafaelfranca
Copy link
Member

:shipit:

@lucasmazza
Copy link
Contributor

👍 :shipit:

josh added a commit that referenced this pull request Mar 24, 2015
Kinda support manifest.compile with simple path
@josh josh merged commit b98efdd into rails:master Mar 24, 2015
@josh josh deleted the detect-nonfnmatch-manifest-compile branch March 24, 2015 02:24
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

Successfully merging this pull request may close these issues.

None yet

3 participants