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

Gitignore #1195

Merged
merged 3 commits into from
Jan 21, 2018
Merged

Gitignore #1195

merged 3 commits into from
Jan 21, 2018

Conversation

gauravtiwari
Copy link
Member

Fixes: #1193

@gauravtiwari gauravtiwari merged commit bf98ffc into master Jan 21, 2018
@gauravtiwari gauravtiwari deleted the gitignore branch January 21, 2018 17:31
kawikadkekahuna added a commit to wealthfit/webpacker that referenced this pull request Jan 29, 2018
* 'master' of https://github.com/rails/webpacker: (21 commits)
  Update Ruby versions on Travis CI (rails#1230)
  Make rubocop happy
  Fix asset helper for non-css assets in hmr mode
  CSP warning for Rails 5.2
  use 2 spaces for indentation in template injection
  Update README.md (rails#1198)
  Remove duplicate yarn-error.log (rails#1197)
  3.2.1
  Revert file loader (rails#1196)
  Update default extensions and move to installer (rails#1181)
  Gitignore (rails#1195)
  Update uglify plugin (rails#1194)
  Fixes Vue root instance initialization (rails#1187)
  Return false in production environments (rails#1179)
  Fix url loader doc[ci skip] (rails#1183)
  Use inject_into_file to support rails 4.2
  Update gems and npm deps (rails#1180)
  asset_pack_path + HMR fix; Closes rails#1171 (rails#1172)
  enhance CHANGELOG.md (rails#1170)
  CI against Ruby 2.5.0
  ...
@@ -27,6 +27,9 @@
/public/packs
/public/packs-test
/node_modules
yarn-debug.log*
yarn-error.log*
.yarn-integrity
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm sorry for coming back to this old issue. However, I have a question about the decision to add .yarn-integrity to the gitignore template file.

So, this means that by default user won't be able to turn on integrity check on their remote server as when webpacker runs yarn check --integrity it would fail.

Is this the intended behavior, e.g. is it recommended to not check this file into source control (despite it seems like it should be shared across machine)?

Thank you.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sikachu No worries! The reason for this is the integrity file would change between environments since the dependencies vary - development vs production(https://yarnpkg.com/lang/en/docs/cli/install/#toc-yarn-install-production-true-false). So, if you run yarn install in development the integrity file would be different vs NODE_ENV=production yarn install (unless I am mistaken).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Production:

{
  "systemParams": "darwin-x64-57",
  "modulesFolders": [
    "node_modules"
  ],
  "flags": [
    "production"
  ],
  "linkedModules": [
    "@rails/webpacker"
  ],

vs

Development:

{
  "systemParams": "darwin-x64-57",
  "modulesFolders": [
    "node_modules"
  ],
  "flags": [],
  "linkedModules": [
    "@rails/webpacker"
  ],

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I see. Thank you very much.

Very interesting, as I thought that yarn check --integrity with .yarn-integrity file is the way to check that the package hasn't been tampered with when you try to install the same package on your production server by comparing to what you have locally, but I guess it's already doing that with packages.json?

Anyway, I guess I'll just turn of the check on production server instead of checking this file in then, with the hope that yarn is already doing the right thing. 💯

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your welcome :)

Yeah, it does that by comparing package.json and yarn.lock file. I guess this check is really helpful in development when you have added some dependencies but forgot to install or using an outdated version as specified in yarn.lock or installed the dependencies in a different environment as the original.

Anyway, I guess I'll just turn off the check on production server instead of checking this file in then, with the hope that yarn is already doing the right thing.

Yeah, you don't need to run this check on production. Haven't run into any problem so far.

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