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

feat: require packageManager in package.json #8017

Merged

Conversation

chris-olszewski
Copy link
Member

Description

With 2.0 we will now be requiring a packageManager field in package.json as this is a best practice and it helps us behave in a deterministic manner.

The actual code change is very straightforward as we remove our package manager inference code and return an error if reading package manager from package.json fails.

Most of the PR is updating tests.

Testing Instructions

Updated unit tests

Copy link

vercel bot commented Apr 22, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
examples-basic-web 🔄 Building (Inspect) Visit Preview 💬 Add feedback Apr 30, 2024 6:54pm
examples-designsystem-docs 🔄 Building (Inspect) Visit Preview 💬 Add feedback Apr 30, 2024 6:54pm
examples-kitchensink-blog 🔄 Building (Inspect) Visit Preview 💬 Add feedback Apr 30, 2024 6:54pm
examples-nonmonorepo ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 30, 2024 6:54pm
examples-svelte-web 🔄 Building (Inspect) Visit Preview 💬 Add feedback Apr 30, 2024 6:54pm
5 Ignored Deployments
Name Status Preview Comments Updated (UTC)
examples-gatsby-web ⬜️ Ignored (Inspect) Visit Preview Apr 30, 2024 6:54pm
examples-native-web ⬜️ Ignored (Inspect) Visit Preview Apr 30, 2024 6:54pm
examples-tailwind-web ⬜️ Ignored (Inspect) Visit Preview Apr 30, 2024 6:54pm
examples-vite-web ⬜️ Ignored (Inspect) Visit Preview Apr 30, 2024 6:54pm
rust-docs ⬜️ Ignored (Inspect) Visit Preview Apr 30, 2024 6:54pm

Copy link
Contributor

github-actions bot commented Apr 22, 2024

🟢 Turbopack Benchmark CI successful 🟢

Thanks

Copy link
Contributor

github-actions bot commented Apr 22, 2024

🟢 CI successful 🟢

Thanks

@chris-olszewski chris-olszewski force-pushed the turborepo_2 branch 3 times, most recently from 7a23a62 to 678e414 Compare April 30, 2024 17:02
@chris-olszewski chris-olszewski force-pushed the chrisolszewski/turbo-2707-require-packagemanager branch from 8e3d15b to 667d38b Compare April 30, 2024 18:54
@chris-olszewski chris-olszewski marked this pull request as ready for review April 30, 2024 20:11
@chris-olszewski chris-olszewski requested a review from a team as a code owner April 30, 2024 20:11
@chris-olszewski chris-olszewski requested review from tknickman and removed request for paulogdm May 8, 2024 15:37
Copy link
Member

@tknickman tknickman left a comment

Choose a reason for hiding this comment

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

Nice!

@chris-olszewski chris-olszewski merged commit 345dbab into turborepo_2 May 9, 2024
55 checks passed
@chris-olszewski chris-olszewski deleted the chrisolszewski/turbo-2707-require-packagemanager branch May 9, 2024 16:12
chris-olszewski added a commit that referenced this pull request May 10, 2024
### Description

With 2.0 we will now be requiring a `packageManager` field in
`package.json` as this is a best practice and it helps us behave in a
deterministic manner.

The actual code change is very straightforward as we remove our package
manager inference code and return an error if reading package manager
from `package.json` fails.

Most of the PR is updating tests.

### Testing Instructions

Updated unit tests
chris-olszewski added a commit that referenced this pull request May 14, 2024
### Description

With 2.0 we will now be requiring a `packageManager` field in
`package.json` as this is a best practice and it helps us behave in a
deterministic manner.

The actual code change is very straightforward as we remove our package
manager inference code and return an error if reading package manager
from `package.json` fails.

Most of the PR is updating tests.

### Testing Instructions

Updated unit tests
chris-olszewski added a commit that referenced this pull request May 20, 2024
### Description

With 2.0 we will now be requiring a `packageManager` field in
`package.json` as this is a best practice and it helps us behave in a
deterministic manner.

The actual code change is very straightforward as we remove our package
manager inference code and return an error if reading package manager
from `package.json` fails.

Most of the PR is updating tests.

### Testing Instructions

Updated unit tests
chris-olszewski added a commit that referenced this pull request May 22, 2024
### Description

With 2.0 we will now be requiring a `packageManager` field in
`package.json` as this is a best practice and it helps us behave in a
deterministic manner.

The actual code change is very straightforward as we remove our package
manager inference code and return an error if reading package manager
from `package.json` fails.

Most of the PR is updating tests.

### Testing Instructions

Updated unit tests
chris-olszewski added a commit that referenced this pull request May 28, 2024
### Description

With 2.0 we will now be requiring a `packageManager` field in
`package.json` as this is a best practice and it helps us behave in a
deterministic manner.

The actual code change is very straightforward as we remove our package
manager inference code and return an error if reading package manager
from `package.json` fails.

Most of the PR is updating tests.

### Testing Instructions

Updated unit tests
chris-olszewski added a commit that referenced this pull request May 29, 2024
### Description

With 2.0 we will now be requiring a `packageManager` field in
`package.json` as this is a best practice and it helps us behave in a
deterministic manner.

The actual code change is very straightforward as we remove our package
manager inference code and return an error if reading package manager
from `package.json` fails.

Most of the PR is updating tests.

### Testing Instructions

Updated unit tests
chris-olszewski added a commit that referenced this pull request May 31, 2024
### Description

With 2.0 we will now be requiring a `packageManager` field in
`package.json` as this is a best practice and it helps us behave in a
deterministic manner.

The actual code change is very straightforward as we remove our package
manager inference code and return an error if reading package manager
from `package.json` fails.

Most of the PR is updating tests.

### Testing Instructions

Updated unit tests
chris-olszewski added a commit that referenced this pull request Jun 4, 2024
### Description

With 2.0 we will now be requiring a `packageManager` field in
`package.json` as this is a best practice and it helps us behave in a
deterministic manner.

The actual code change is very straightforward as we remove our package
manager inference code and return an error if reading package manager
from `package.json` fails.

Most of the PR is updating tests.

### Testing Instructions

Updated unit tests
chris-olszewski added a commit that referenced this pull request Jul 12, 2024
### Description

Give users the ability to opt out of the required package manager if
they feel strongly.

This is still highly discouraged as this prevents proper usage of daemon
package watching and leaves us dependent on system configuration to
infer the correct package manager.

Reviewer notes: Each commit can be reviewed on it's own. The second
commit just adds back code deleted in
#8017

### Testing Instructions

Added unit tests where applicable for configuration.

Quick manual verification of the config options:

```
[0 olszewski@chriss-mbp] /tmp/no-pm $ turbo_dev build                                                                
 WARNING  No locally installed `turbo` found. Using version: 2.0.7-canary.0.
  × missing packageManager field in package.json

[1 olszewski@chriss-mbp] /tmp/no-pm $ turbo_dev build --dangerously-disable-package-manager-check --output-logs=none
 WARNING  No locally installed `turbo` found. Using version: 2.0.7-canary.0.
• Packages in scope: @repo/eslint-config, @repo/typescript-config, @repo/ui, docs, web
• Running build in 5 packages
• Remote caching disabled

 Tasks:    2 successful, 2 total
Cached:    2 cached, 2 total
  Time:    81ms >>> FULL TURBO

[0 olszewski@chriss-mbp] /tmp/no-pm $ TURBO_DANGEROUSLY_DISABLE_PACKAGE_MANAGER_CHECK=true turbo_dev build --output-logs=none                                           
 WARNING  No locally installed `turbo` found. Using version: 2.0.7-canary.0.
• Packages in scope: @repo/eslint-config, @repo/typescript-config, @repo/ui, docs, web
• Running build in 5 packages
• Remote caching disabled

 Tasks:    2 successful, 2 total
Cached:    2 cached, 2 total
  Time:    210ms >>> FULL TURBO

[0 olszewski@chriss-mbp] /tmp/no-pm $ vim turbo.json                                                                         
[0 olszewski@chriss-mbp] /tmp/no-pm $ tail -3 turbo.json 
  },
  "dangerouslyDisablePackageManagerCheck": true
}
[0 olszewski@chriss-mbp] /tmp/no-pm $ turbo_dev build --output-logs=none                                                    
 WARNING  No locally installed `turbo` found. Using version: 2.0.7-canary.0.
• Packages in scope: @repo/eslint-config, @repo/typescript-config, @repo/ui, docs, web
• Running build in 5 packages
• Remote caching disabled

 Tasks:    2 successful, 2 total
Cached:    2 cached, 2 total
  Time:    53ms >>> FULL TURBO
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants