Skip to content
This repository has been archived by the owner on Aug 31, 2022. It is now read-only.

concat ASI and non ASI libraries #11

Closed
staabm opened this issue Oct 29, 2012 · 7 comments
Closed

concat ASI and non ASI libraries #11

staabm opened this issue Oct 29, 2012 · 7 comments

Comments

@staabm
Copy link
Contributor

staabm commented Oct 29, 2012

when concatenating js files which use Automatic Semicolon Insertion with other which doesn't use ASI syntax errors may occur.

I could fix those errors, just by adding a ";" between the concatenated files.

    $contents .= $newcontents;
    if( $ftype === "js" ){
                // add a extra semicolon, so ASI and non-ASI js-files while not clash
        $contents .= ';';
    }

The console shows erorrs like TypeError: (intermediate value)(...) is not a function, which do not appear when loading the files using separate <scritpt> tags

@jefflembeck
Copy link
Collaborator

I'd really prefer not adding something like this. Quickconcat has the purpose of concatenating your files, but it doesn't edit them.

@scottjehl , what do you think?

@scottjehl
Copy link
Member

I don't have a strong opinion, but I think this is fairly common in JS concatenation scripts.

I'd support the addition if others think it's valuable.

On Oct 29, 2012, at 5:51 PM, Jeff Lembeck wrote:

I'd really prefer not adding something like this. Quickconcat has the purpose of concatenating your files, but it doesn't edit them.

@scottjehl , what do you think?


Reply to this email directly or view it on GitHub.

@jefflembeck
Copy link
Collaborator

Ah, if it's common (and therefore expected), then I'm cool with it.

@scottjehl
Copy link
Member

We already muck with file paths anyway, so I wouldn't say we're very hands-off with contents to begin with.

On Oct 29, 2012, at 6:02 PM, Jeff Lembeck wrote:

Ah, if it's common (and therefore expected), then I'm cool with it.


Reply to this email directly or view it on GitHub.

@staabm
Copy link
Contributor Author

staabm commented Oct 30, 2012

I also think it's a common case that some do ASI and some not..
e.g. github uses ASI -> https://github.com/defunkt/jquery-pjax
while jQuery doesn't.

if we go in this direction, maybe we should rewrite the util in a more object oriented way so we will not mix up all the filetype specific stuff...

if so, in the git repos we should have one class per file and utilize a build mechanism which produces the 1-file util we have today..

@jefflembeck
Copy link
Collaborator

Let's split this into two issues. One to address the ASI/non-ASI problem and the other to discuss how the util is written. It's a pretty big change and would require tests all along the way.

@staabm
Copy link
Contributor Author

staabm commented Oct 30, 2012

Great, thanks! 👍

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

No branches or pull requests

3 participants