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.

Sign up for GitHub

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

Jump to bottom

Use @sylius-ui/frontend npm package #871

Merged
merged 1 commit into from
Dec 13, 2022
Merged

Use @sylius-ui/frontend npm package #871

merged 1 commit into from
Dec 13, 2022

Conversation

jakubtobiasz
Copy link
Contributor

@jakubtobiasz jakubtobiasz requested a review from a team as a code owner November 10, 2022 16:11
@vvasiloi
Copy link
Contributor

Since all dependencies are now devDependencies, are there any limitations users should be aware of?

@jakubtobiasz
Copy link
Contributor Author

Nope, the whole application is built, so none dependency is used later.

@Ferror
Copy link
Contributor

Ferror commented Nov 15, 2022

I don't understand why the Sylius frontend is dev dependency

@jakubtobiasz
Copy link
Contributor Author

@Ferror notice all gulp, webpack, and others were in dev section. None of these packages are required by runtime, as they all are only used to build the result files. All of these deps can be placed in both dependencies and devDependencies (I mean the result will be the same) so as more deps were in dev then I placed this package as dev dependency too.

@Ferror
Copy link
Contributor

Ferror commented Nov 16, 2022

@Ferror notice all gulp, webpack, and others were in dev section. None of these packages are required by runtime, as they all are only used to build the result files. All of these deps can be placed in both dependencies and devDependencies (I mean the result will be the same) so as more deps were in dev then I placed this package as dev dependency too.

Yea they were all in devDependency and then run with GULP_ENV=prod.

@jakubtobiasz
Copy link
Contributor Author

@Ferror notice all gulp, webpack, and others were in dev section. None of these packages are required by runtime, as they all are only used to build the result files. All of these deps can be placed in both dependencies and devDependencies (I mean the result will be the same) so as more deps were in dev then I placed this package as dev dependency too.

Yea they were all in devDependency and then run with GULP_ENV=prod.

Now they are yarn build:prod what under the hood is yarn encore production, I don't see any difference.

Rafikooo
Rafikooo approved these changes Nov 29, 2022
View reviewed changes
Copy link
Contributor

@Rafikooo Rafikooo left a comment

Choose a reason for hiding this comment

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

I'm wondering if we should open the new branch (1.13-dev) as these changes are targeting Sylius 1.13

@jakubtobiasz jakubtobiasz force-pushed the feature/use-sylius-ui branch 2 times, most recently from 627b44c to 39ed2d9 Compare December 8, 2022 12:14
@Rafikooo Rafikooo merged commit 67896b9 into Sylius:1.12 Dec 13, 2022
@Rafikooo
Copy link
Contributor

Thank you, Jakub! 🥇

@jakubtobiasz jakubtobiasz deleted the feature/use-sylius-ui branch December 13, 2022 10:47
frozenpixel added a commit to frozenpixel/Sylius-Celletech that referenced this pull request Jul 7, 2023
@frozenpixel
Merge tag 'v1.12.3' into celletech-1.12.2
fe030f8 
v1.12.3

- [Sylius#873](Sylius#873) [maintenance] - Ignore yarn.lock file ([@Ferror](https://github.com/Ferror))
- [Sylius#861](Sylius#861) [docker] - Bug fix for cron container ([@Ferror](https://github.com/Ferror))
- [Sylius#860](Sylius#860) [docker] - Enable cron container to wait for migrations ([@Ferror](https://github.com/Ferror))
- [Sylius#871](Sylius#871) Use @sylius-ui/frontend npm package ([@jakubtobiasz](https://github.com/jakubtobiasz))
- [Sylius#891](Sylius#891) [Maintenance] PHP Attributes in the Entity classes ([@Rafikooo](https://github.com/Rafikooo))
- [Sylius#855](Sylius#855) [Composer] Remove composer.lock from .gitignore after creating the project ([@artyuum](https://github.com/artyuum))
- [Sylius#899](Sylius#899) Fix builds ([@coldic3](https://github.com/coldic3))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@NoResponseMate NoResponseMate NoResponseMate approved these changes

@coldic3 coldic3 coldic3 approved these changes

@Rafikooo Rafikooo Rafikooo approved these changes

Assignees
No one assigned
Labels
DX Frontend
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@jakubtobiasz @vvasiloi @Ferror @Rafikooo @NoResponseMate @coldic3