search

skatejs / web-components

[DEPRECATED] - The frictionless way to use the webcomponents/webcomponentsjs polyfills.
MIT License
17 stars 4 forks source link

chore: fix backtick escape replace #41

Closed elmariofredo closed 7 years ago

elmariofredo commented 7 years ago

Closes #40

elmariofredo commented 7 years ago

DO NOT MERGE! Resolving CI issue https://travis-ci.org/skatejs/web-components/builds/204153459#L433 which explains why it didn't fail on this issue

elmariofredo commented 7 years ago

So I have find out that onBuildEnd was silently failing see log as it was missing dist folder at time of copying. As a result we were missing native-shim.js in released packages. I suspect that this is culprit https://github.com/skatejs/build/blob/master/bin/sk-bundle#L3 but I couldn't reproduce it on local.

What is more important this fix doesn't work on linux new escaping will deform native-shim.js.

So I'm closing this PR and I think that proper solution to this issue would be

  1. create backtick replace loader( should be really simple https://github.com/webpack-contrib/raw-loader/blob/master/index.js#L8)
  2. use https://github.com/kevlened/copy-webpack-plugin to copy native-shim.js
micahscopes commented 7 years ago

@elmariofredo I'm having a hard time understanding point (1)

A backtick replace loader? Is the only point of that sed command to replace backticks with escaped backticks? (Trying to understand all those \\\'s 😮 ) Would the raw-loader just be using javascript to do what the sed command is doing? I don't think I understood that reference to https://github.com/webpack-contrib/raw-loader/blob/master/index.js#L8

elmariofredo commented 7 years ago

@micahscopes yes exactly it make \` from ` so whole native-shim file can be then encapsulated in backticks.

My point about loader is to create custom loader to replace backticks. Reference to raw-loader is only as an example how easy it would be to create replace-backtick-loader

Simply said we need some multi platform solution to this as according to my experience sed doesn't work the same even across POSIX.

micahscopes commented 7 years ago

@elmariofredo alright, that's exactly what I figured... doesn't seem so bad!

elmariofredo commented 7 years ago

@micahscopes great! let me know if you will need help with this.

Cheers 🍻

treshugart commented 7 years ago

sed was just an easy implementation. Can write a node script or whatever to do this if we need to.

micahscopes commented 7 years ago

I'm working on it now, should have it done after lunch. Just gonna make a simple webpack plugin that should fit in the webpack config.