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

Already on GitHub? Sign in to your account

CssCompressor doesn't support @import #341

Open
Lacrymology opened this Issue Dec 5, 2012 · 5 comments

Comments

Projects
None yet
2 participants

if a css file has @import in it, the resulting concatenated file fails because @import can only be at the top of the file.

I did the following, but it's a horrible fix because it makes preprocessing and HDD read happen twice. Luckily, this only happens once per file compilation.

class CssCompressor(Compressor):
   #....
    def split_contents(self):
        if self.split_content:
            return self.split_content
        self.media_nodes = []
        for elem in self.parser.css_elems():
            data = None
            elem_name = self.parser.elem_name(elem)
            elem_attribs = self.parser.elem_attribs(elem)
            start_new = False
            if elem_name == 'link' and elem_attribs['rel'].lower() == 'stylesheet':
                basename = self.get_basename(elem_attribs['href'])
                filename = self.get_filename(basename)
                data = (SOURCE_FILE, filename, basename, elem)
                ################### THIS IS THE NEW CODE
                options = {
                    'method': METHOD_INPUT,
                    'elem': elem,
                    'kind': data[0],
                    'basename': basename,
                    }

                options = dict(options, filename=filename)
                value = self.get_filecontent(filename, self.charset)

                if self.all_mimetypes:
                    precompiled, value = self.precompile(value, **options)
                start_new = '@import' in value
                ###############################

            elif elem_name == 'style':
                data = (SOURCE_HUNK, self.parser.elem_content(elem), None, elem)
            if data:
                self.split_content.append(data)
                media = elem_attribs.get('media', None)
                # Append to the previous node if it had the same media type
                append_to_previous = self.media_nodes and self.media_nodes[-1][0] == media
                # and we are not just precompiling, otherwise create a new node.
                if ((not start_new) and
                    append_to_previous and
                    settings.COMPRESS_ENABLED):
                    self.media_nodes[-1][1].split_content.append(data)
                else:
                    node = SafeCssCompressor(content=self.parser.elem_str(elem),
                                         context=self.context)
                    node.split_content.append(data)
                    self.media_nodes.append((media, node))
        return self.split_content
  #...
Owner

diox commented Dec 12, 2012

FWIW, it's a duplicate of #231, which is already closed. The docs mention this and recommends to avoid @import.

Very nice, but django's own admin CSS uses @import..

Owner

diox commented Dec 12, 2012

Django admin doesn't use compressor by default, you need some template modifications everywhere to make it work. Since you need those template modifications anyway, you can work around the issue by writing a small filter that removes all imports and manually add imported files to your templates. This is actually what I ended up doing to have compressor support in our customized admin at @liberation.

I do agree this is isn't optimal and it's just a workaround, but we don't have a proper solution for the moment.

I made those modifications everywhere already in order to use sekizai for it (because there's other issues with multiple jquery instances if some apps you use are not up-to-date with current admin templates), so it was just natural to compress stuff.

The only solution I can see that wil avoid reading the file twice, really, is to expand the @import'ed file into the compressed concatenated file.

Owner

diox commented Dec 14, 2012

Yep, it seems doable as a filter (not enabled by default) expanding @imported content. It's tricky, because you want to support @imports inside @imports, but it's doable.

@diox diox added the feature label Sep 26, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment