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

Twig 2 upgrade (Drupal 9 compatibility) #2258

Merged
merged 27 commits into from
Sep 24, 2021
Merged

Conversation

remydenton
Copy link
Collaborator

@remydenton remydenton commented Jul 6, 2021

Jira

https://pegadigitalit.atlassian.net/browse/DS-489

Summary

Updates pattern Lab to twig 2.0 for compatibility with Drupal 9

Details

  • Fixes deprecated/improper twig usages in Bolt source code so that it's compatible with Twig 2.
  • Patches/hacks pattern lab php for compatibility with twig 2.

How to test

  • On this branch, confirm that anywhere twig is installed in a vendor directory (vendor/twig/twig), version 2.x is being used. The quickest way to do this is by looking at vendor/twig/twig/CHANGELOG and confirming the latest lines references twig 2.14.
  • Confirm no regressions in pattern lab: https://feature-ds-489-twig-2.boltdesignsystem.com/

Release notes

Support for twig 2 added.

You can't patch dependency versions with a composer patch, so this never
actually worked.  See https://github.com/cweagans/composer-patches#patching-composerjson-in-dependencies
Dot notation does some fancy magic behind the scenes.  Read more here:
https://twig.symfony.com/doc/2.x/templates.html

Something that was added in Twig 2.x, when using foo.bar, it will check
for a `hasBar` method on the underlying object if `bar` is not a property
of it.  In this case, `hasClass` is a method, but not one we intend to
ever call.  This syntax makes that clear to twig, I think.
See https://twig.symfony.com/doc/1.x/deprecated.html#macros

> As of Twig 2.0, macros imported in a file are not available in child
> templates anymore (via an include call for instance). You need to
> import macros explicitly in each file where you are using them.
…g 2.x"

This reverts commit 875f426b0d7557e583ea8469bf81dd4fd0f8bb7c.
The implemented/extended classes here were deprecated well before this
code was originally written in 2018, in
#426.

In any case, \Twig\Extension\GlobalsInterface is needed to for the
getGlobals() method to work now.
See https://twig.symfony.com/doc/2.x/advanced.html
@github-actions github-actions bot added the type: feature List this PR in the 'Features' section of the release notes. label Jul 6, 2021
@colbytcook colbytcook temporarily deployed to feature/DS-489-twig-2--branch-preview July 6, 2021 22:20 Inactive
@colbytcook colbytcook temporarily deployed to feature/DS-489-twig-2--branch-preview July 12, 2021 22:43 Inactive
@colbytcook colbytcook changed the title DO NOT MERGE Twig 2 upgrade (Drupal 9 compatibility) [DO NOT MERGE] Twig 2 upgrade (Drupal 9 compatibility) Aug 12, 2021
basaltinc/twig-tools should use the master branch of our fork at
https://github.com/boltdesignsystem/twig-tools.  Updated last in
boltdesignsystem/twig-tools#1

bolt-design-system/core-php should use the latest version of that
project which will be deployed to https://github.com/boltdesignsystem/core-php
(and packagist) when this branch is included in a Bolt release.  It's
annoying circular; we can't actually test that until we cut the release.
These changes are copied from this commit in the read-only repo, that
was done temporarily while testing:
boltdesignsystem/bolt_connect@fe5e99d
' Conflicts:
'	packages/testing/testing-helpers/package.json
'	packages/twig-integration/drupal-module/composer.json
'	packages/twig-integration/twig-renderer/package.json
@remydenton remydenton changed the title [DO NOT MERGE] Twig 2 upgrade (Drupal 9 compatibility) Twig 2 upgrade (Drupal 9 compatibility) Sep 22, 2021
@colbytcook colbytcook had a problem deploying to feature/DS-489-twig-2--branch-preview September 22, 2021 20:31 Failure
@colbytcook colbytcook requested a deployment to feature/DS-622-rebuild-hero-no-wc--f2b61b6f--commit-preview September 22, 2021 21:03 In progress
@colbytcook colbytcook requested a deployment to feature/ds-590-page-header-notifications--f4c658b1--commit-preview September 24, 2021 16:37 In progress
@colbytcook
Copy link
Contributor

colbytcook commented Sep 24, 2021

@remydenton approved, I did not notice any regressions with these changes.

  • Commands tested
    yarn
    yarn setup
    yarn start
    yarn test:js
    yarn cc
    yarn ce
    npx jest /Users/cookc/dev/bolt/packages/elements/bolt-bbb/__tests__/bbb.js -u
    npx jest /Users/cookc/dev/bolt/packages/components/bolt-aa/__tests__/aa.js -u
  • Travis checks completed successfully
  • Visual Regression Test

Copy link
Collaborator

@danielamorse danielamorse left a comment

Choose a reason for hiding this comment

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

  • Reviewed the code as much as I could. See just what I'd expect to see here.
  • Spot-checked PL and everything looks good.
  • Checked out this branch and got twig 2 installed on the first yarn clean && yarn setup. Perhaps it worked for me because I'd already cleared cache previously.

@colbytcook colbytcook merged commit 5c7440a into master Sep 24, 2021
@colbytcook colbytcook deleted the feature/DS-489-twig-2 branch September 24, 2021 17:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: feature List this PR in the 'Features' section of the release notes.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants