How to upgrade to 1.0.0

[bitbager]

ocramius/proxy-manager package is locked to ^1.0|^2.0 but the composer.json has ^2.1 requirement. 2.1 version causes Warning: Cannot bind closure to scope of internal class ReflectionProperty while running behat tests.

[stefandoorn]

Actually I'm also experiencing this when using kernel inside KernelTestCase in unit / integration tests.

[lchrusciel]

Are you sure it is needed? I have updated one project and several plugins(including ShoApiPlugin) The main problem was cache. Also, TestKernelClass has changed

[bitbager]

Clearing the cache does not solve the tests issue. I talked to @pamil on Slack and he recommended downgrading the ocramius/proxy-manage to 2.0.

[lchrusciel]

Hmm.. Funny thing. It is not something I will fight for, so I'm good with it. Just saying that working with 2.1 was fine for me. Doesn't matter if it was a Sylius-Standard upgrade or ShopApiPlugin.

[bitbager]

My plugins are also working with 2.1, the problem appears in platform projects 🙁

[bitbager]

Ok, I figured it out. This issue is caused by the test environment difference between Sylius-standard from the current version and previous ones. Below is the quick tutorial how to fix this issue as well as migrate to new Sylius Standard from the previous version.

1. cd /path/to/project
2. $ git remote add upstream https://github.com/Sylius/Sylius-Standard.git
3. git checkout dev
4. git fetch --all
5. git pull upstream master
6. At this point probably many conflicts will appear. Unfortunately, much of them would need manual resolving. In case of files which were never touched during the project, you can copy the https://github.com/Sylius/Sylius-Standard content. This might include files like AppCache.php, TestAppKernel.php, etc.
7. After resolving all issues, run $ composer update
8. Run $ bin/behat & $ bin/phpspec run to fix all issues caused by new implementation.

Below are some common issues I faced:

1. Model classes now return ArrayCollections instead of arrays, so if you want to mock it in PHPSpec, you need to wrap mocked object like this:

    function it_does_something(
        ReviewableInterface $reviewable,
        ReviewInterface $review
    ): void {
        $reviewable->getReviews()->willReturn(new ArrayCollection([$review->getWrappedObject()]));
    }

2. Due to new Twig version, macros in included templates need to be imported in the child template. See https://stackoverflow.com/questions/41590051/twig-2-0-error-message-accessing-twig-template-attributes-is-forbidden/41590052

3. If you don't know it yet, new Sylius supports only PHP 71 which enables you to use scalar types. This may require adding declare(strict_types=1); declaration to your PHP files, especially to customized models, and defining the parameter and returned types. See more on the begin of http://php.net/manual/de/migration71.new-features.php manual page.

4. Sylius requires now Symfony 3.3.8. Personally, I didn't find any problems with upgrading, but some things have changed between 3.2 and 3.3. The one I now remind is different cache management, so you may need to clear it more often, even in the dev environment.

This answer is written at 4 AM, the night before the stable release. Please, forgive me if there are any mistakes in it.

I will try to create a blog post with a recommended deploy way, just to save time other folks. From my point of view, Sylius 1.0.0 is ready to be shown to the world 🎉 .

Great work guys!

[lchrusciel]

Mikołaj, it was 4 AM. We could wait for this upgrade guide :) Anyway, thanks a lot for it.

[lchrusciel]

Also, it is probably also a good starting point for others to share common problems or tips during the upgrade. And we will avoid the mess

[stefandoorn]

git fetch upstream is enough, no need to fetch other remotes :) Small note.

[lchrusciel]

I would personally advise to not pull sylius standard directly to your app, but rather update sylius/sylius and see what is broken. From my perspective, what you need to check are:

1. TestAppKernel - it has been changed - probably should be copied from sylius/sylius
2. If you have type hints for arrays it is possible that you will have to change them to iterable as we are using collections
3. It can happen that you have type-hinted to the core class, but interface requires class from another bundle, so just adjust namespaces.

The projects that I have been involved in recently contains strict types from the day one, so update was pretty straight forward. But if you don't have them yet it can make you some troubles.

[stefandoorn]

Your suggested flow worked 99% for me, but I have some unit tests failing with the same issue @bitbager experiences. I don't see it happening local, only on GitLab CI test runs currently. I'm trying now an upgrade as @bitbager suggested, to see if these changes around testing will resolve the issue. If the file needs to be copied, might be good to mention that in UPGRADE.md. Until now that seems to be the biggest change in the conflicts I'm resolving right now indeed.

[lchrusciel]

I have seen a similar problem with Warning: Cannot bind closure to scope of internal class ReflectionProperty but it happens only on Travis-CI. For me clearing the cache solved it.

[pamil]

@bitbager I've suggested upgrading it to ^2.0, so 2.1 does also apply, because I thought you had ^1.0 installed. Anyway, clearing the cache will be enough for Warning: Cannot bind closure to scope of internal class ReflectionProperty error then.

@lchrusciel

If you have type hints for arrays it is possible that you will have to change them to iterable as we are using collections

Our models does not usually return iterable, we had changed array to Collection.

[stefandoorn]

I shouldn't cheer too early, but it seems that merging the latest changes from Sylius-Standard did the job. One of the specific unit tests that yesterday failed is now passing in my CI runs.

I mainly think it's the TestAppKernel that was the cause why I got this error, didn't have the latest version (still the version of beta-1).

Clearing cache didn't help though without copying that TestAppKernel file, @pamil.

[stale[bot]]

This issue has been automatically marked as stale because it has not had any recent activity. It will be closed in a week if no further activity occurs. Thank you for your contributions.