Skip to content

[Docs] Add step to disable package.json synchronization while upgrading PHP package #2698

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

Open
wants to merge 1 commit into
base: 2.x
Choose a base branch
from

Conversation

Kocal
Copy link
Member

@Kocal Kocal commented Apr 21, 2025

Q A
Bug fix? no
New feature? no
Docs? yes
Issues Fix #...
License MIT

Following symfony/symfony#58678 (comment)

@carsonbot carsonbot added docs Improvements or additions to documentation Status: Needs Review Needs to be reviewed labels Apr 21, 2025
@Kocal Kocal requested review from kbond and smnandre April 21, 2025 06:54
@carsonbot carsonbot added Status: Reviewed Has been reviewed by a maintainer and removed Status: Needs Review Needs to be reviewed labels Apr 21, 2025
@@ -14,6 +14,18 @@ composer require symfony/ux-autocomplete:2.23.0
npm add @symfony/[email protected]
```

To prevent your `package.json` file from being overwritten by Symfony Flex when upgrading the PHP package, you must **use at least Symfony Flex ^1.22.0 or ^2.5.0**, and configure your `composer.json` file like this:
Copy link
Contributor

Choose a reason for hiding this comment

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

What about using the github markdown note ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Does it work on NPM website?

Copy link
Contributor

Choose a reason for hiding this comment

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

I honestly dont know (same for packagist and other sf php repo readme)
But at least Github display info is clearer and denote this specific paragraph

@smnandre
Copy link
Member

I'm not sure here... as always by fear to make newcomers make mistake ... once this is enabled, installing packages will not work anymore, and then no documentation would help :|

@Kocal
Copy link
Member Author

Kocal commented Apr 25, 2025

I'm not sure here... as always by fear to make newcomers make mistake ... once this is enabled, installing packages will not work anymore, and then no documentation would help :|

I see what you mean, but remember our UX npm packages are reserved for advanced users only, not newcomers.

@smnandre
Copy link
Member

The problem is want to avoid is someone having no idea locally why they cannot install an UX package anymore.

I do believe this is something we should address asap..

In the meantime i understand the need, so can we just add a warning ? Something like "Please be aware NONE of your installed paclages NOR the next you would install .... " (or something like that) ?

@Kocal
Copy link
Member Author

Kocal commented Apr 26, 2025

The problem is want to avoid is someone having no idea locally why they cannot install an UX package anymore.

A warning is displayed to the user when synchronize_package_json is false: https://github.com/symfony/flex/blob/8ce1acd9842abe0e9b4c4a0bd3f259859516c018/src/Flex.php#L506

It should be enough no?

In the meantime i understand the need, so can we just add a warning ? Something like "Please be aware NONE of your installed paclages NOR the next you would install .... " (or something like that) ?

I will rewrite the paragraph to be more explicit about that

@Kocal Kocal force-pushed the doc-npm-pkg-disable-pkg.json-sync branch from 5b4dab0 to b3a15b4 Compare April 26, 2025 09:38
@Kocal
Copy link
Member Author

Kocal commented Apr 26, 2025

PR updated

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Improvements or additions to documentation Status: Reviewed Has been reviewed by a maintainer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants