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

Reference replacement mistake #97

Open
th0r opened this issue Jun 22, 2015 · 11 comments
Open

Reference replacement mistake #97

th0r opened this issue Jun 22, 2015 · 11 comments

Comments

@th0r
Copy link

th0r commented Jun 22, 2015

  1. Make file app.js with the following code:
var el = document.getElementById('app');
  1. Revision this script using gulp and the following task:
gulp.task('hash-app', () => {
    var RevAll = require('gulp-rev-all');
    var revAll = new RevAll();

    return gulp
        .src('app.js')
        .pipe(revAll.revision())
        .pipe(gulp.dest('.'));
});
  1. As result app.c8c8c872.js file appears with the following content:
var app = document.getElementById('app.c8c8c872');

Seems like the plugin thinks that 'app' in document.getElementById('app') is a reference and replaces it.

@circlingthesun
Copy link
Contributor

You can specify an annotator and replacer to resolve these issues. The annotator splits your files into subsections that can be annotated and the replacer does the reference replacement taking into account the annotations. See my comment near the bottom of #86 for an implementation.

@circlingthesun
Copy link
Contributor

@th0r
Copy link
Author

th0r commented Jun 22, 2015

@circlingthesun Is there any way to just say hey, don't be too clever and replace only filenames with extensions?

@circlingthesun
Copy link
Contributor

Not without modifying the code. At the moment the regular expressions are used to find potential references and then they are reused to replace them. So what you need to do is modify the regular expression that is used to replace matched references so that the extension is included for .js files. Specifying this as the replacer should work:

var replacer = function(fragment, replaceRegExp, newReference, referencedFile){
    var regExp =  replaceRegExp;;

    if(referencedFile.path.match(/.js$/)){
       // Make the extension manditory
        var regExpParts = replaceRegExp.toString().split('/');
        var opts = regExpParts.pop();
        regExpParts.splice(0, 1); // Remove the leading slash
        var regexStr = regExpParts.join('\/').replace('(\\.js)?', '(\\.js)');
        regExp = new RegExp(regexStr, opts);   
    }
    fragment.contents = fragment.contents.replace(regExp, '$1' + newReference + '$3$4');
};

@th0r
Copy link
Author

th0r commented Jun 22, 2015

@circlingthesun Pain...but thanks

@circlingthesun
Copy link
Contributor

Yeah, I've been meaning to write a better interface to this but I've not found the time.

@sixinli
Copy link

sixinli commented Nov 3, 2015

would love to have an easier way to only replace matches with file extension, but thanks for the example!

@dietrich-stein
Copy link

Great module! Also, 👍 for an extensions option.

@werts
Copy link

werts commented Feb 20, 2017

great extension at all, but what i have got a wrong replace,for i had adding annotator and replacer,it doesnt work exactly at all, inside annotator,i have added angular.module to identify a module within angular.

annotator: function(contents, path) {
                var moduleLoaderRegex = /(?:define|angular\.module|require|System\.register|System\.import)\s*\(\s*(?:(?:['"][^'"]*['"]\s?,\s?)?(?:\[[^\]]*|(?:function))|(?:['"][^'"]*['"]\s?))/g;
                var safeShortSubList = [];
                var result;

                while (result = moduleLoaderRegex.exec(contents)) {
                    if (!result) {
                        continue;
                    }
                    safeShortSubList.push(result[0]);
                }

some mistakes look like this:

"use strict";angular.module("app.18126505").controller("AppCtrl"

and the rest is the same with yours.

@cbfranca
Copy link

I'm with one similar problem and honestly i don't understand how can do that. (#177 )

@curiousercreative
Copy link

It's not obvious from the responses above, but the following disables this feature altogether it seems:

  RevAll.revision({
    replacer: function () {}
  })

@circlingthesun it seems this function would only operate on javascript files to start (no stylesheets) and it's a side-effects only function so this seems safe if we only want to rewrite the filenames in the filesystem but not our compiled code.

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

7 participants