fix issue/61: split('/n') does not work on Windows #89
Conversation
|
@dnalborczyk: Thank you for submitting a pull request! Before we can merge it, you'll need to sign the Meteor Contributor Agreement here: https://contribute.meteor.com/ |
src/index.js
Outdated
| @@ -1,3 +1,4 @@ | |||
| const os = require('os'); | |||
There was a problem hiding this comment.
@dnalborczyk will this work in the browser environment? Does webpack polyfill this when prepublish is run?
There was a problem hiding this comment.
the 'os' module does work in webpack. it's using: https://github.com/webpack/node-libs-browser, which in turn uses: https://github.com/CoderPuppy/os-browserify. that being said, I'm not so sure anymore if that is the right approach for src/index.js, since the new line escape sequence is always '\n', no matter which browser. though if src/index.js is only meant to run from within webpack, it might as well be fine. I can also revert the PR for src/index.js if it's more suitable ...
|
"src/index" gets run on the client as well, whereas the loader only gets
run in Node. Do you mind adjusting "src/index" to not use "os" so we don't
have to ship extra code to browsers?
Thank you for your contribution!
…On Mon, May 29, 2017 at 3:16 PM dnalborczyk ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In src/index.js
<#89 (comment)>
:
> @@ -1,3 +1,4 @@
+const os = require('os');
the 'os' module does work in webpack. it's using:
https://github.com/webpack/node-libs-browser, which in turn uses:
https://github.com/CoderPuppy/os-browserify. that being said, I'm not so
sure anymore if that is the right approach for src/index.js, since the new
line escape sequence is always '\n', no matter which browser. if
src/index.js is only meant to run from within webpack, it might as well be
fine.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#89 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABERRpMtwH3WUHnp4JZIFSAfH__pegerks5r-0PNgaJpZM4NppQ1>
.
|
|
I reverted the changes, though I thought src/index.js is running in the browser exclusively? |
|
src/index will get compiled and dist/index will get sent to clients to parse GraphQL documents at runtime. the only time that this gets called on the server at compile-time is when used with the webpack loader (which is thank you @dnalborczyk! |
this should fix #61