-
-
Notifications
You must be signed in to change notification settings - Fork 93
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
feat!: always use package_json
for managing node packages
#430
Changes from all commits
e38e764
b668371
18408b6
680e86e
ac20d2e
c676b25
5052248
8dc4e5c
e9c3b21
5ba68f8
58c0ff1
b7c42db
a67041a
c99bf5b
26d537d
6515c73
cab244d
bf5691a
791cc5a
f08a313
f66292a
e6ae3c0
9032d08
c073e1e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -51,8 +51,6 @@ Read the [full review here](https://clutch.co/profile/shakacode#reviews?sort_by= | |
- [Optional support](#optional-support) | ||
- [Installation](#installation) | ||
- [Rails v6+](#rails-v6) | ||
- [Using alternative package managers](#using-alternative-package-managers) | ||
- [Note for Yarn v2 usage](#note-for-yarn-v2-usage) | ||
- [Concepts](#concepts) | ||
- [Usage](#usage) | ||
- [Configuration and Code](#configuration-and-code) | ||
|
@@ -101,13 +99,13 @@ Read the [full review here](https://clutch.co/profile/shakacode#reviews?sort_by= | |
- Ruby 2.7+ | ||
- Rails 5.2+ | ||
- Node.js 14+ | ||
- Yarn | ||
|
||
## Features | ||
- Rails view helpers that fully support Webpack output, including HMR and code splitting. | ||
- Convenient but not required webpack configuration. The only requirement is that your webpack configuration creates a manifest. | ||
- HMR with the `shakapacker-dev-server`, such as for hot-reloading React! | ||
- Automatic code splitting using multiple entry points to optimize JavaScript downloads. | ||
- Support for [NPM](https://www.npmjs.com/package/npm), Yarn ([classic](https://classic.yarnpkg.com/lang/en/) and [berry](https://yarnpkg.com/getting-started)), [PNPM](https://pnpm.io/), and [Bun](https://bun.sh/) | ||
G-Rath marked this conversation as resolved.
Show resolved
Hide resolved
|
||
- [Webpack v5+](https://webpack.js.org/) | ||
- ES6 with [babel](https://babeljs.io/), [SWC](https://swc.rs/), or [Esbuild](https://github.com/privatenumber/esbuild-loader) | ||
- Asset compression, source-maps, and minification | ||
|
@@ -147,40 +145,45 @@ Then run the following to install Shakapacker: | |
|
||
Before initiating the installation process, ensure you have committed all the changes. While installing Shakapacker, there might be some conflict between the existing file content and what Shakapacker tries to copy. You can either approve all the prompts for overriding these files or use the `FORCE=true` environment variable before the installation command to force the override without any prompt. | ||
|
||
When `package.json` and/or `yarn.lock` changes, such as when pulling down changes to your local environment in team settings, be sure to keep your NPM packages up-to-date: | ||
Shakapacker uses the [`package_json`](https://github.com/shakacode/package_json) gem to handle updating the `package.json` and interacting with the underlying package manager of choice for managing dependencies and running commands; the package manager is managed using the [`packageManager`](https://nodejs.org/api/packages.html#packagemanager) property in the `package.json`, otherwise falling back to the value of `PACKAGE_JSON_FALLBACK_MANAGER` if set or otherwise `npm`. | ||
G-Rath marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
```bash | ||
yarn | ||
``` | ||
|
||
Note, in v6+, most JS packages are peer dependencies. Thus, the installer will add the packages: | ||
If `packageManager` is not set when running `shakapacker:install`, Shakapacker will set it based on the lockfile and the result of calling `--version` on the inferred manager; if no lockfile is present, then `npm` be used unless you choose to explicitly set the `PACKAGE_JSON_FALLBACK_MANAGER` to your preferred package manager. | ||
|
||
```bash | ||
yarn add @babel/core @babel/plugin-transform-runtime @babel/preset-env @babel/runtime babel-loader \ | ||
compression-webpack-plugin terser-webpack-plugin \ | ||
webpack webpack-assets-manifest webpack-cli webpack-merge webpack-sources webpack-dev-server | ||
``` | ||
|
||
Previously, these "webpack" and "babel" packages were direct dependencies for `shakapacker`. By | ||
making these peer dependencies, you have control over the versions used in your webpack and babel configs. | ||
|
||
### Using alternative package managers | ||
|
||
There is experimental support for using package managers besides Yarn classic for managing JavaScript dependencies using the [`package_json`](https://github.com/G-Rath/package_json) gem. | ||
|
||
This can be enabled by setting the environment variable `SHAKAPACKER_USE_PACKAGE_JSON_GEM` to `true`; Shakapacker will then use the `package_json` gem which in turn will look for the [`packageManager`](https://nodejs.org/api/packages.html#packagemanager) property in the `package.json` or otherwise the `PACKAGE_JSON_FALLBACK_MANAGER` environment variable to determine which manager to use, defaulting to `npm` if neither are found. | ||
> **Note** | ||
> | ||
> The `packageManager` property is only used to determine the package manager to use, based primarily on its name. | ||
> The version (if present) is only used to determine if Yarn Classic or Yarn Berry should be used, but is otherwise | ||
> _not_ checked, nor is [`corepack`](https://nodejs.org/api/corepack.html) used to ensure that the package manager is installed. | ||
> | ||
> It is up to the developer to ensure that the desired package manager is actually install at the right version, which can be done | ||
> using `corepack` or by other means. | ||
|
||
See [here](https://github.com/G-Rath/package_json#specifying-a-package-manager) for a list of the supported package managers and more information; note that `package_json` does not handle ensuring the manager is installed. | ||
|
||
> **Note** | ||
> | ||
> The rest of the documentation assumes that `package_json` is not being used, and so always references `yarn` - you should instead use the package manager of your choice for these commands. | ||
> [NOTE] | ||
> | ||
> The rest of the documentation will only reference `npm` when providing commands such as to install optional packages except in cases where | ||
> a particular package manager requires a very different command; otherwise it should be safe to just replace `npm` with the name of your | ||
> preferred package manager when running the command | ||
|
||
### Note for Yarn v2 usage | ||
Note, in v6+, most JS packages are peer dependencies. Thus, the installer will add the packages: | ||
|
||
If you are using Yarn v2 (berry), please note that PnP modules are not supported unless you're using `SHAKAPACKER_USE_PACKAGE_JSON_GEM`. | ||
G-Rath marked this conversation as resolved.
Show resolved
Hide resolved
|
||
- `@babel/core` | ||
- `@babel/plugin-transform-runtime` | ||
- `@babel/preset-env` | ||
- `@babel/runtime` | ||
- `babel-loader` | ||
- `compression-webpack-plugin` | ||
- `terser-webpack-plugin` | ||
- `webpack` | ||
- `webpack-assets-manifest` | ||
- `webpack-cli` | ||
- `webpack-merge` | ||
- `webpack-sources` | ||
- `webpack-dev-server` | ||
|
||
To use Shakapacker with Yarn v2, make sure you set `nodeLinker: node-modules` in your `.yarnrc.yml` file as per the [Yarn docs](https://yarnpkg.com/getting-started/migration#step-by-step) to opt out of Plug'n'Play behavior. | ||
Previously, these "webpack" and "babel" packages were direct dependencies for `shakapacker`. By | ||
making these peer dependencies, you have control over the versions used in your webpack and babel configs. | ||
|
||
## Concepts | ||
|
||
|
@@ -621,13 +624,13 @@ See also [Customizing Babel Config](./docs/customizing_babel_config.md) for an e | |
#### TypeScript | ||
|
||
```bash | ||
yarn add typescript @babel/preset-typescript | ||
npm install typescript @babel/preset-typescript | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Talked briefly with @justin808 and in two minds about this default. I hear the point around matching node default (since you will always get at least NPM), but think Rails has mostly set on using yarn and requires bit of extra work to get other package managers working. How hard would it be to retain yarn default? Is adding pkg manager into package.json something we could automate? Some existing tool detection akin to https://github.com/rails/jsbundling-rails/blob/main/lib/tasks/jsbundling/build.rake? As said elsewhere, switching manager after install shouldn't be a rocked science so not opposed to new default in principle, but given the ecosystem, I wonder if we can make upgrade less annoying (those 5 mins switching back to yarn and having package-lock.json when I've had yarn before WILL annoy me 😅) or find some other compromise? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If we don't find a good compromise path though, I would say that IMO benefits of choice of pkg manager outweigh the pain of switching There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
From what I've seen that was covered by Webpacker which is now Shakapacker, and so because that now supports multiple package managers there is no "bit of extra work" required (beyond setting up the package manager of choice, which isn't our cost to pay) - it's literally set the The thing that makes it tricky is The reason why
This is imo the hardest thing to address because unlike It might be ok though 🤔 This has also inspired me to have another look at the |
||
``` | ||
|
||
Babel won’t perform any type-checking on TypeScript code. To optionally use type-checking run: | ||
|
||
```bash | ||
yarn add fork-ts-checker-webpack-plugin | ||
npm install fork-ts-checker-webpack-plugin | ||
``` | ||
|
||
Add tsconfig.json | ||
|
@@ -668,7 +671,7 @@ module.exports = generateWebpackConfig({ | |
To enable CSS support in your application, add the following packages: | ||
|
||
```bash | ||
yarn add css-loader style-loader mini-css-extract-plugin css-minimizer-webpack-plugin | ||
npm install css-loader style-loader mini-css-extract-plugin css-minimizer-webpack-plugin | ||
``` | ||
|
||
Optionally, add the `CSS` extension to webpack config for easy resolution. | ||
|
@@ -692,18 +695,18 @@ then add the relevant pre-processors: | |
#### Postcss | ||
|
||
```bash | ||
yarn add postcss postcss-loader | ||
npm install postcss postcss-loader | ||
``` | ||
|
||
Optionally add these two plugins if they are required in your `postcss.config.js`: | ||
```bash | ||
yarn add postcss-preset-env postcss-flexbugs-fixes | ||
npm install postcss-preset-env postcss-flexbugs-fixes | ||
``` | ||
|
||
#### Sass | ||
|
||
```bash | ||
yarn add sass-loader | ||
npm install sass-loader | ||
``` | ||
|
||
You will also need to install [Dart Sass](https://github.com/sass/dart-sass), [Node Sass](https://github.com/sass/node-sass) or [Sass Embedded](https://github.com/sass/embedded-host-node) to pick the implementation to use. sass-loader will automatically pick an implementation based on installed packages. | ||
|
@@ -712,35 +715,35 @@ Please refer to [sass-loader documentation](https://www.npmjs.com/package/sass-l | |
|
||
##### Dart Sass | ||
```bash | ||
yarn add sass | ||
npm install sass | ||
``` | ||
|
||
##### Node Sass | ||
```bash | ||
yarn add node-sass | ||
npm install node-sass | ||
``` | ||
|
||
##### Sass Embedded | ||
```bash | ||
yarn add sass-embedded | ||
npm install sass-embedded | ||
``` | ||
|
||
#### Less | ||
|
||
```bash | ||
yarn add less less-loader | ||
npm install less less-loader | ||
``` | ||
|
||
#### Stylus | ||
|
||
```bash | ||
yarn add stylus stylus-loader | ||
npm install stylus stylus-loader | ||
``` | ||
|
||
#### CoffeeScript | ||
|
||
```bash | ||
yarn add coffeescript coffee-loader | ||
npm install coffeescript coffee-loader | ||
``` | ||
|
||
#### Other frameworks | ||
|
@@ -840,16 +843,24 @@ bundle update shakapacker | |
# overwrite your changes to the default install files and revert any unwanted changes from the install | ||
rails shakapacker:install | ||
|
||
# yarn 1 instructions | ||
# using npm | ||
npm install shakapacker@latest | ||
npm install webpack-dev-server@latest | ||
|
||
# using yarn classic | ||
yarn upgrade shakapacker --latest | ||
yarn upgrade webpack-dev-server --latest | ||
|
||
# yarn 2 instructions | ||
# using yarn berry | ||
yarn up shakapacker@latest | ||
yarn up webpack-dev-server@latest | ||
|
||
# using pnpm | ||
pnpm up shakapacker@latest | ||
pnpm up webpack-dev-server@latest | ||
|
||
# Or to install the latest release (including pre-releases) | ||
yarn add shakapacker@next | ||
npm install shakapacker@next | ||
Comment on lines
-852
to
+863
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The Though not related to this PR, it is worth it to review it again. CC: @justin808 |
||
``` | ||
|
||
Also, consult the [CHANGELOG](./CHANGELOG.md) for additional upgrade links. | ||
|
@@ -916,7 +927,24 @@ Shakapacker hooks up a new `shakapacker:compile` task to `assets:precompile`, wh | |
|
||
This behavior is optional & can be disabled by either setting a `SHAKAPACKER_PRECOMPILE` environment variable to `false`, `no`, `n`, or `f`, or by setting a `shakapacker_precompile` key in your `shakapacker.yml` to `false`. ([source code](./lib/shakapacker/configuration.rb#L34)) | ||
|
||
When compiling assets for production on a remote server, such as a continuous integration environment, it's recommended to use `yarn install --frozen-lockfile` to install NPM packages on the remote host to ensure that the installed packages match the `yarn.lock` file. | ||
When compiling assets for production on a remote server, such as a continuous integration environment, it's recommended to ensure the exact versions specified in your lockfile are installed: | ||
|
||
``` | ||
# using npm | ||
npm ci | ||
|
||
# using yarn classic | ||
yarn install --frozen-lockfile | ||
|
||
# using yarn berry | ||
yarn install --immutable | ||
|
||
# using pnpm | ||
pnpm install --frozen-lockfile | ||
|
||
# using bun | ||
bun install --frozen-lockfile | ||
``` | ||
|
||
If you are using a CDN setup, Shakapacker does NOT use the `ASSET_HOST` environment variable to prefix URLs for assets during bundle compilation. You must use the `SHAKAPACKER_ASSET_HOST` environment variable instead (`WEBPACKER_ASSET_HOST` if you're using any version of Webpacker or Shakapacker before Shakapacker v7). | ||
|
||
|
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.
Curious on
npm
default rationale - how did we land here? Traditionallyyarn
was the only one supported so this feels like a fairly inconvenient change for upgrade potentially. Is it just the nature ofpackage-json
gem?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.
Because that aligns with the default for Node /
package.json
-npm
is the default package manager that ships with Node and so in the absence of apackageManager
inpackage.json
, is what gets used; that's also whynpm
is not actually listed as a supported package manager withcorepack
.Really the biggest impact of this should be when installing packages, which as of v8 we won't do automatically - running commands with a different package manager is usually fine (I use my
nrun
script locally for running commands which uses alwaysnpm
under the hood, and not had any issues), so this should primarily come up when you're doingshakapacker:install
which is typically at the start of a fresh project so should just mean having to spend 5 minutes figuring out how to switch back to Yarn if that's what you want.I do have an idea on a way we might be able to handle this a bit more gracefully but it would add a bunch of extra faff and I'm keen to get us being package manager agnostic by default so I'd rather go ahead with this and then later follow-up with the "graceful improvement" in a minor version if we do get feedback that highlights it worth the faff.
I will make it clearer here though that people should ensure
packageManager
is set as part of the upgradeThere 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'm still not a fan of switching away from
yarn
as the default unless there's a good reason.https://chat.openai.com/share/d33fe865-defd-41df-ac89-4db307a71052
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.
That AI conversation is not really useful because it doesn't have the full context - ultimately shakapacker itself shouldn't be setting a default as it's a library, and so
npm
is the default because that's the default in Node which is the default runtime shakapacker targets.This change means effectively shakapacker aligns with node in terms of switching package managers so if you want to use yarn you just follow the recommended steps (i.e. corepack and
packageManager
) and both Node and shakapacker will respect that and just work 🙂The only place we're selecting
npm
as an opinion is in the docs but that's because I don't think it's worth the overhead of documenting every possible combination of every package manager 🤷♂️