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 node path usage #322

Merged
merged 2 commits into from
Aug 21, 2017
Merged

Fix node path usage #322

merged 2 commits into from
Aug 21, 2017

Conversation

stoikerty
Copy link
Contributor

Hey Jared,

On my exploration through razzle I stumbled over this compatibility issue. The culprit is a race-condition which I fixed by replacing the use of the process.env.NODE_PATH-global with an exported variable. Not sure why it only happens on windows, must have something to do with how Node handles imports/exports and envs in that OS.

It was already working like this:

// packages/razzle/config/createConfig.js
// ...
const getEnv = require('./env');

const nodePath = process.env.NODE_PATH;

module.exports = (
// ...
      modules: ['node_modules', paths.appNodeModules].concat(
        nodePath.split(path.delimiter).filter(Boolean)
      )
// ...

but I thought that having a dangling variable there would be confusing, therefore I opted to reduce the reliance on the relevant global entirely.

Love the concept of razzle btw, great stuff 👍

@jaredpalmer jaredpalmer merged commit b3916f0 into jaredpalmer:master Aug 21, 2017
@jaredpalmer
Copy link
Owner

Yasss! This is dope.

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