Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

[FrameworkBundle] changed some default configs from canBeEnabled to canBeDisabled#21196

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
fabpot merged 2 commits intosymfony:masterfromfabpot:can-be-disabled
Jan 8, 2017

Conversation

@fabpot
Copy link
Member

@fabpotfabpot commentedJan 7, 2017
edited
Loading

QA
Branch?master
Bug fix?no
New feature?yes
BC breaks?no
Deprecations?no
Tests pass?yes
Fixed ticketsn/a
LicenseMIT
Doc PRn/a

FrameworkBundle configuration is currently "optimized" for when a project
depends on symfony/symfony (which is the vast majority of Symfony projects out
there as that's how Symfony Standard Edition is set up). As all components are
always available, features (forms, validation, translation, serializer, ...)
are disabled by default in FrameworkBundle's configuration (canBeEnabled) and
developers must enable them when they need them (that was done mainly for
performance reasons).

That's annoying as it means one configuration step before being able to use
forms for the first time (or validation, or serialization, or translation, ...).

To make features auto-configurable and make the framework a bit more
user-friendly (that's where I think I need to invoke DX :), I'd like Symfony 4
to work in a different way. Instead of relying on symfony/symfony, I want
people to install only the components/bundles they need. In that scenario, we
can auto-configure Symfony FrameworkBundle based on the available components.
If you add symfony/form as a dependency, then it makes sense to automatically
enable the feature in framework bundle (with the possibility to disable it if
needed thanks tocanBeDisabled).

Let's recap:

  • Before:

    • You want to use forms; you have symfony/symfony, so nothing to install;

    • Using forms does not work out of the box though; you need to know that you
      have to editapp/config/config.yml to explicitly enable forms;

    • Forms work!

  • After:

    • You want to use forms so you install symfony/form (like for any other
      packages out there; want to use Twig for templating, install twig/twig);

    • But for Symfony components, there are no other steps; forms are
      auto-configured just because you installed the dependency, go work now!

In a way, it makes handling/installing/configuring Symfony components no
different than doing the same for a third party package. That's about relying
even more on Composer and less on configuration. Symfony components have the
extra benefit of being auto-configured via FrameworkBundle. That's not the case
for other third-party packages/bundles, but for those who attended SymfonyCon
Berlin, you know that this is coming soon via Symfony Flex.

That's even more interesting for forms as CSRF protection needs an extra knob
to be turned on currently. With the new way, just install the CSRF security
component. An again, you still have the possibility to turn it off if you want
to.

Anyway, this PR gives us the flexibility to do both: when using
symfony/symfony, everything works as before, if you are using
symfony/framework-bundle, then auto-configuration based on the installed
packages is automatically activated.

This also brings consistency as this behavior is already what we've done for
the Doctrine Annotation library in 3.2.

Last, but not the least, with all the work currently done on the container
lazyness for Symfony 3.3, concerns about performance are less important than
before, so having components auto-enabled when installed should not be a big
deal. We might even go one step further and remove enabling/disabling for
ESI/SSI/fragments/... And whenever we create an independent Session component,
we will be able to do the same with the session configuration. The astute
reader might have noticed that I haven't talked about the templating
configuration, but as the component will be deprecated in 3.3, I prefer to keep
its activation explicit.

In terms of BC, the only change is for people using symfony/framework-bundle
with some packages that were installed but not enabled in the config. I would
say that this should be pretty rare and anyway, the only consequence is a small
performance hit which can be easily offset by explicitly disabling the config.

That's all folks!

linaori, ogizanagi, chalasr, and sstok reacted with thumbs up emojidunglas and HeahDude reacted with hooray emoji
@nicolas-grekas
Copy link
Member

👍

@fabpotfabpotforce-pushed thecan-be-disabled branch 3 times, most recently from8a1caa9 to5c1f9a1CompareJanuary 7, 2017 17:58
->children()
->arrayNode('csrf_protection')
->canBeEnabled()
->{!class_exists(FullStack::class) &&class_exists(CsrfToken::class) ?'canBeDisabled' :'canBeEnabled'}()
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't we add a new method toArrayNodeDefinition instead ?
Like

->canBeAutomaticallyEnabled(function() {return !class_exists(FullStack::class) &&class_exists(CsrfToken::class);}

Copy link
MemberAuthor

Choose a reason for hiding this comment

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

I don't think this is needed as this is rarely useful.

GuilhemN reacted with thumbs up emoji
@wouterj
Copy link
Member

I like this way :)

Btw, it's quite funny, as Symfony changed everything to disabled in Symfony 3:#13703

Koc and sstok reacted with laugh emoji

@dunglas
Copy link
Member

👍

@fabpotfabpotforce-pushed thecan-be-disabled branch 3 times, most recently from15ced32 to3539248CompareJanuary 7, 2017 22:00
@fabpotfabpot merged commitef80873 intosymfony:masterJan 8, 2017
fabpot added a commit that referenced this pull requestJan 8, 2017
…nBeEnabled to canBeDisabled (fabpot)This PR was squashed before being merged into the 3.3-dev branch (closes#21196).Discussion----------[FrameworkBundle] changed some default configs from canBeEnabled to canBeDisabled| Q             | A| ------------- | ---| Branch?       | master| Bug fix?      | no| New feature?  | yes| BC breaks?    | no| Deprecations? | no| Tests pass?   | yes| Fixed tickets | n/a| License       | MIT| Doc PR        | n/aFrameworkBundle configuration is currently "optimized" for when a projectdepends on symfony/symfony (which is the vast majority of Symfony projects outthere as that's how Symfony Standard Edition is set up). As all components arealways available, features (forms, validation, translation, serializer, ...)are disabled by default in FrameworkBundle's configuration (`canBeEnabled`) anddevelopers must enable them when they need them (that was done mainly forperformance reasons).That's annoying as it means one configuration step before being able to useforms for the first time (or validation, or serialization, or translation, ...).To make features auto-configurable and make the framework a bit moreuser-friendly (that's where I think I need to invoke DX :), I'd like Symfony 4to work in a different way. Instead of relying on symfony/symfony, I wantpeople to install only the components/bundles they need. In that scenario, wecan auto-configure Symfony FrameworkBundle based on the available components.If you add symfony/form as a dependency, then it makes sense to automaticallyenable the feature in framework bundle (with the possibility to disable it ifneeded thanks to `canBeDisabled`).Let's recap: * Before:   * You want to use forms; you have symfony/symfony, so nothing to install;   * Using forms does not work out of the box though; you need to know that you     have to edit `app/config/config.yml` to explicitly enable forms;   * Forms work! * After:   * You want to use forms so you install symfony/form (like for any other     packages out there; want to use Twig for templating, install twig/twig);   * But for Symfony components, there are no other steps; forms are     auto-configured just because you installed the dependency, go work now!In a way, it makes handling/installing/configuring Symfony components nodifferent than doing the same for a third party package. That's about relyingeven more on Composer and less on configuration. Symfony components have theextra benefit of being auto-configured via FrameworkBundle. That's not the casefor other third-party packages/bundles, but for those who attended SymfonyConBerlin, you know that this is coming soon via Symfony Flex.That's even more interesting for forms as CSRF protection needs an extra knobto be turned on currently. With the new way, just install the CSRF securitycomponent. An again, you still have the possibility to turn it off if you wantto.Anyway, this PR gives us the flexibility to do both: when usingsymfony/symfony, everything works as before, if you are usingsymfony/framework-bundle, then auto-configuration based on the installedpackages is automatically activated.This also brings consistency as this behavior is already what we've done forthe Doctrine Annotation library in 3.2.Last, but not the least, with all the work currently done on the containerlazyness for Symfony 3.3, concerns about performance are less important thanbefore, so having components auto-enabled when installed should not be a bigdeal. We might even go one step further and remove enabling/disabling forESI/SSI/fragments/... And whenever we create an independent Session component,we will be able to do the same with the session configuration. The astutereader might have noticed that I haven't talked about the templatingconfiguration, but as the component will be deprecated in 3.3, I prefer to keepits activation explicit.In terms of BC, the only change is for people using symfony/framework-bundlewith some packages that were installed but not enabled in the config. I wouldsay that this should be pretty rare and anyway, the only consequence is a smallperformance hit which can be easily offset by explicitly disabling the config.That's all folks!Commits-------ef80873 [FrameworkBundle] changed some default configs from canBeEnabled to canBeDisabled98ce21a [FrameworkBundle] changed the default value of annotation setting based on the existence of Doctrine Annotations
nicolas-grekas added a commit that referenced this pull requestJan 8, 2017
This PR was merged into the 3.3-dev branch.Discussion----------[TwigBundle] Disable form in tests| Q             | A| ------------- | ---| Branch?       | master| Bug fix?      | no| New feature?  | no| BC breaks?    | no| Deprecations? | no| Tests pass?   | yes| Fixed tickets | n/a| License       | MIT| Doc PR        | n/aTests are broken with high deps due to the changes made in#21196> Symfony\Bundle\TwigBundle\Tests\NoTemplatingEntryTest::testSymfony\Component\DependencyInjection\Exception\LogicException: Form support cannot be enabled as the Translation component is not installed.> Symfony\Bundle\TwigBundle\Tests\NoTemplatingEntryTest::testSymfony\Component\DependencyInjection\Exception\LogicException: The Validator component is required to use the Form component.This will fix them.Commits-------2a279b9 [TwigBundle] Disable form in tests
@fabpotfabpot deleted the can-be-disabled branchJanuary 8, 2017 21:24
@fabpotfabpot mentioned this pull requestMay 1, 2017
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

1 more reviewer

@GuilhemNGuilhemNGuilhemN left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@fabpot@nicolas-grekas@wouterj@dunglas@GuilhemN@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp