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

fix: explicitly set our preferred package manager for js dependencies #528

Merged
merged 1 commit into from
Jan 26, 2024

Conversation

G-Rath
Copy link
Contributor

@G-Rath G-Rath commented Jan 25, 2024

react-rails now supports other package managers using package_json, meaning it now defaults to npm if a package manager is not explicitly set in package.json, causing a stray package-lock.json to be created.

I've specified an exact version because currently corepack requires that (ranges are not yet supported) but we're not actually expecting anyone to be using corepack and we're definitely not using it in production (not explicitly at least...) so in theory this shouldn't cause problems and it means package.json conforms to the JSON schema - it'll also help to flush out any potential problems early before(/if) corepack becomes more mainstream.

Copy link
Contributor

@joshmcarthur joshmcarthur left a comment

Choose a reason for hiding this comment

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

Approved, with the standard observation that it would be nice to be able to put comments into package.json to explain why we do things like an exact version lock

@G-Rath G-Rath force-pushed the explicitly-set-package-manager branch from 03b5cd1 to 68c3768 Compare January 26, 2024 02:28
@G-Rath G-Rath force-pushed the explicitly-set-package-manager branch from 68c3768 to c24e2bf Compare January 26, 2024 02:28
@G-Rath G-Rath merged commit 5a59ebc into main Jan 26, 2024
22 checks passed
@G-Rath G-Rath deleted the explicitly-set-package-manager branch January 26, 2024 19:13
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.

4 participants