X Tutup
Skip to content
This repository was archived by the owner on Apr 8, 2020. It is now read-only.

FIxing path separator to address #247#248

Merged
SteveSandersonMS merged 2 commits intoaspnet:devfrom
aaronpowell:node-path-error
Aug 8, 2016
Merged

FIxing path separator to address #247#248
SteveSandersonMS merged 2 commits intoaspnet:devfrom
aaronpowell:node-path-error

Conversation

@aaronpowell
Copy link
Member

@aaronpowell aaronpowell commented Aug 7, 2016

When investigating #247 I noticed that the NODE_PATH would look something like:

C:\tmp\.nvm\v6.3.1\node_modules:C:\_code\project\node_modules

The problem is that the path separator is : not ;.

This fix have resolve the issue in my testing.

Testing fix

  1. Create a new project using the yeoman generator
  2. Enable server side rendering
  3. Set NODE_PATH to have a value (eg: C:\tmp)
  4. Run the application and experience the error described in Can't find aspnet-prerendering when using a node version manager #247
  5. Apply patch
  6. Run the application successfully

@dnfclas
Copy link

dnfclas commented Aug 7, 2016

Hi @aaronpowell, I'm your friendly neighborhood .NET Foundation Pull Request Bot (You can call me DNFBOT). Thanks for your contribution!
You've already signed the contribution license agreement. Thanks!

The agreement was validated by .NET Foundation and real humans are currently evaluating your PR.

TTYL, DNFBOT;

@aaronpowell
Copy link
Member Author

aaronpowell commented Aug 7, 2016

Hmm just noticed that : is what *nix seems to use as a path separator (testing using Bash on Ubuntu on Windows). My primary dev environment is Windows.

@aaronpowell
Copy link
Member Author

aaronpowell commented Aug 7, 2016

Fixing to use Path.PathSeparator like it should do to pick ; or :.

@SteveSandersonMS SteveSandersonMS merged commit 2a6465b into aspnet:dev Aug 8, 2016
@SteveSandersonMS
Copy link
Member

SteveSandersonMS commented Aug 8, 2016

Fantastic! Thanks @aaronpowell for tracking this down and preparing a nice clean fix :) It's really great to have this one resolved.

@aaronpowell aaronpowell deleted the node-path-error branch August 8, 2016 03:16
@aaronpowell
Copy link
Member Author

aaronpowell commented Aug 8, 2016

@SteveSandersonMS what's the release model for this? Wanting to pull it into some samples 😉

@SteveSandersonMS
Copy link
Member

SteveSandersonMS commented Aug 8, 2016 •
edited
Loading

@aaronpowell I've pushed an updated set of beta packages (NodeServices, SpaServices, etc., all versioned 1.0.0-beta-000010), so your fix should be present in any new projects you create, or in existing ones if you force it to update the NuGet packages. Thanks again for the PR!

@SteveSandersonMS SteveSandersonMS mentioned this pull request Aug 8, 2016
Closed
This was referenced Jul 17, 2020
Open
Open
@snyk-bot snyk-bot mentioned this pull request Sep 2, 2021
Open
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@aaronpowell @dnfclas @SteveSandersonMS
X Tutup