-
Notifications
You must be signed in to change notification settings - Fork 750
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
Updating install docs #682
Conversation
@@ -43,7 +43,9 @@ npm install --save luxon | |||
const { DateTime } = require("luxon"); | |||
``` | |||
|
|||
If you want to work with locales, you'll need to have `full-icu` support installed in Node. You can [build Node with it](https://github.com/nodejs/node/wiki/Intl), use an [NPM module](https://www.npmjs.com/package/full-icu) to provide it, or find it prepackaged for your platform, like `brew install node --with-full-icu`. If you skip this step, Luxon still works but methods like `setLocale()` will do nothing. | |||
If you want to work with locales, you'll need to have `full-icu` support installed in Node. You can use an [NPM module](https://www.npmjs.com/package/full-icu) to provide it, or find it prepackaged in Node 13+. Please note that trying to install node via brew using `brew install node --with-full-icu` does not work - you're much better off using [nvm](https://github.com/nvm-sh/nvm). If you want to use a Node version prior to 13 _and_ have full icu support, you'll need to build from source using `nvm install <version> -s --with-intl=full-icu --download=all`. If you skip this step, Luxon still works but methods like `setLocale()` will do nothing. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. I've always used the homebrew one. Why doesn't it work?
I'm not loving the explicit recommendation of nvm, since there are tons of tools out there. I think giving the example and keeping agnostic is the best way. Finally, I think it might be better to present this as tree, something like:
- If you're using Node 13+, ICU is prepackaged in the default build and no further action is required
- If you are using an earlier version of node, you must either:
A. Install a build of Node with full ICU baked in, such as via nvm:nvm install <version> -s --with-intl=full-icu --download=all
or brew:brew install node --with-full-icu
B. Install the ICU data externally and point Node to it. The instructions on how to do that are below.
If you skip this step, Luxon still works but methods like setLocale()
will do nothing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know, it just doesn't. I get an error saying that the argument isn't valid.
I can definitely present this a bit differently, making nvm out to be one of a choice (using your language of "such as via x".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was also on the nodejs irc channel as part of my debugging this issue, and they recommended I use the nvm versions rather than the Homebrew versions, and it served me well, so I figured the same recommendations should probably apply here.
If you want to work with locales, you'll need to have `full-icu` support installed in Node. You can [build Node with it](https://github.com/nodejs/node/wiki/Intl), use an [NPM module](https://www.npmjs.com/package/full-icu) to provide it, or find it prepackaged for your platform, like `brew install node --with-full-icu`. If you skip this step, Luxon still works but methods like `setLocale()` will do nothing. | ||
If you want to work with locales, you'll need to have `full-icu` support installed in Node. You can use an [NPM module](https://www.npmjs.com/package/full-icu) to provide it, or find it prepackaged in Node 13+. Please note that trying to install node via brew using `brew install node --with-full-icu` does not work - you're much better off using [nvm](https://github.com/nvm-sh/nvm). If you want to use a Node version prior to 13 _and_ have full icu support, you'll need to build from source using `nvm install <version> -s --with-intl=full-icu --download=all`. If you skip this step, Luxon still works but methods like `setLocale()` will do nothing. | ||
|
||
Please note that the warning above for Node applies when trying to run tests for your web app - those tests will be run using the local default Node version. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess I don't quite understand this clarification. What confusion is this clearing up?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was getting terribly confused before this all started as to why I was getting different results in my tests and in my app, until I remembered that the tests use Node to run, and my app uses webpack. This is more of a piece of text meant to stimulate an 'aha!' moment as people (like my stupid self) might not realise or be too blinkered to step back and realise. I can take it out if you think it isn't necessary, but I think it's useful. Up to you :)
Updating the install docs on the luxon website to improve instructions for installing full icu with Node.
I'm more than happy to take suggestions about the wording, but I wanted to make it much more clear what the landscape is now with current versions of Node and how Homebrew works. Have just spent an afternoon dealing with this so figured I'd update the docs so someone else doesn't have to. :)