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

Fix Webpack conf #3690

Merged
merged 3 commits into from
Apr 22, 2017
Merged

Fix Webpack conf #3690

merged 3 commits into from
Apr 22, 2017

Conversation

t3chguy
Copy link
Member

@t3chguy t3chguy commented Apr 21, 2017

Fixes:

ERROR in ./~/olm/olm.js
Module not found: Error: Cannot resolve module 'fs' in C:\Users\t3chg\WebStormProjects\riot-web\node_modules\olm
 @ ./~/olm/olm.js 18:39-52

+ conform to a bit more eslint (IDE makes my eyes hurt)
+ specify windows-specific copies of noParse regexes to stop the olm error

Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>
Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>
@ara4n
Copy link
Member

ara4n commented Apr 22, 2017

hm, by switching forward slashes for backwards slashes, doesn't this break things on unix in favour of working on windows? or is there some voodoo happening to make it work on unix too that i'm missing?

also, i'm not keen on some of the formatting changes here (or mixing them together with the functional bit of the commit) - the long lines have been deliberately broken up in places for aesthetic clarity. (or has eslint bullied you into this?)

@t3chguy
Copy link
Member Author

t3chguy commented Apr 22, 2017

It isn't switching forward slashes for backwards slashes, its providing a regex for each, so one would trigger on *nix and the other on Windows. If any regex trips then that file won't be parsed.

ESLint very much bullied me complaining about a lacking trailing comma if its split across multiple lines, I trusted its false positives in the first commit and thus the test failed spectacularly.

@t3chguy
Copy link
Member Author

t3chguy commented Apr 22, 2017

though the more readable approach is probably to use a character class with both forward and backward slashes and an appropriate comment [\\\/]

its more readable than:

new RegExp(path.join('highlight\.js', 'lib', 'languages'))

add a comment so this madness has meaning

Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>
@ara4n
Copy link
Member

ara4n commented Apr 22, 2017

oh, sorry - i completely missed that it was adding the backslash regexp rather than replacing the fwdslash one. The character class feels miles better and is more DRYing- thanks.

@ara4n ara4n merged commit 634cf52 into element-hq:develop Apr 22, 2017
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

Successfully merging this pull request may close these issues.

2 participants