Esri / esri-wab-build

Package used to build ESRI Web App Builder Apps for production.
Apache License 2.0
17 stars 5 forks source link

writeProfile in prebuild.js strips regex #36

Open lancegosby opened 7 years ago

lancegosby commented 7 years ago

when writing the app.profile.js file, the prebuild.js script uses JSON.stringify(). Unfortunately this strips out any regex from the profile and replaces it with an empty object.

In my _app.profile.js file, I am trying to fix the build errors thrown by moment by excluding some files from the package using the trees property. For example:

packages: [
    { 
        //other packages
    },
    {
      name: "moment",
      location: "./moment",
      trees: [
        [".", ".", /(\/\.)|(~$)|(test|txt|src|min|templates)/]
      ]
    },
    {
        // more packages
    }
]
Biboba commented 6 years ago

Hi @lancegosby,

Did you find a workaround to this issue ? I can't make a build work because of this.

Thanks

lancegosby commented 6 years ago

I did this a while ago so not sure if I changed anything else. In prebuild.js, I updated the writeProfile function as follows:

function writeProfile() {

  function replacer(key, value) {
    // wrap RegExp so we can find later
    if (value instanceof RegExp) {
      return ("__REGEXP " + value.toString() +  " REGEXP__");
    }
    else
      return value;
  }

  // use replacer function to wrap any RegEx so we can find it
  var tmpProfileStr = "profile = " + JSON.stringify(profile, replacer, 2) + ";";
  // convert back to RegEx before writing to file
  var profileStr = tmpProfileStr.replace(/"__REGEXP (.*) REGEXP__"/g, function(match, p1) {
    // unescape backslashes since this was stringified
    return p1.replace(/\\\\/g,"\\");
  });
  fs.writeFileSync(wProfileFile, profileStr, "utf-8");
}
Biboba commented 6 years ago

Many thanks @lancegosby ! It worked : from 193 errors, I am now at 24

build always "fails"

gbochenek commented 6 years ago

Good stuff @lancegosby ! Would it be less confusing to swap out RegExp.toJSON over this text replace?

lancegosby commented 6 years ago

@gbochenek I'm not sure exactly what you mean. I remember trying a few different things at the time I was working through this. Not sure why I stuck with this, but it works for us. If you can improve on it, or make it less confusing, I would interested to see it.

gbochenek commented 6 years ago

I definitely think its a good solution.

For incorporating this as a pull request, I'm wondering if something like the solution here: https://stackoverflow.com/questions/12075927/serialization-of-regexp/22571090#22571090 would save other devs from having to use some serialization constant (___REGEXP) in the JSON, but I could also see pitfalls there.

What are your thoughts?

On Thu, Apr 19, 2018 at 1:49 PM Lance Gosby notifications@github.com wrote:

@gbochenek https://github.com/gbochenek I'm not sure exactly what you mean. I remember trying a few different things at the time I was working through this. Not sure why I stuck with this, but it works for us. If you can improve on it, or make it less confusing, I would interested to see it.

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/gbochenek/esri-wab-build/issues/36#issuecomment-382823617, or mute the thread https://github.com/notifications/unsubscribe-auth/AB1zZpyaT9g40yuxlD7OJQDf1CSNJ0HIks5tqM4RgaJpZM4PBvh_ .

lancegosby commented 6 years ago

You don't need to put "__REGEXP" in your _app.profile.js file at all. You can see a snippet of my file at the top of this issue.

I think the problem with RegExp.prototype.toJSON = RegExp.prototype.toString; is that the toString function escapes the backslashes and puts the whole regex into a string. The Dojo build trees property doesn't work with a string. It needs to be a RegExp object.

The replacer function wraps the RegExp string so we can undo those unwanted side effects.

Unfortunately, I don't have time to create a pull request at the moment.

gbochenek commented 6 years ago

Makes sense now. I need to read more carefully next time!

If anyone has time, I would be happy to accept a pull request for this solution.

rsjones commented 4 years ago

@lancegosby Do you know if this work around is still needed with newer version of WAB?