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

[careful] handlebars 3+4 #243

Merged
merged 9 commits into from
Jul 1, 2016
Merged

[careful] handlebars 3+4 #243

merged 9 commits into from
Jul 1, 2016

Conversation

matteofigus
Copy link
Member

@matteofigus matteofigus commented May 5, 2016

This is not easy. Handlebars 3 has some security issues (#252).

Unfortunately upgrading to v4 is not a piece of cake, as compiled templates with v3, are not going to be compatible with parser v4. This means that published v3 components are not going to be executable anymore with v4 and they need a new compilation phase.

This means that for a graceful migration, we will

  • Introduce Handlebars 4 in this PR as default compiler for new components. This will include the Handlebars 3 polyfill so that pre-compiled templates with previous version will keep working.
  • Publish oc with a new minor version so that all the components in a registry will be republished after being recompiled with H4
  • Work on a following PR to remove Handlebars 3 polyfill support as a cleaning step + new oc minor publish.

@matteofigus matteofigus added the wip label May 5, 2016
@matteofigus matteofigus changed the title [breaking change] handlebars 4 [breaking change - don't merge yet] handlebars 4 May 5, 2016
@matteofigus matteofigus changed the title [breaking change - don't merge yet] handlebars 4 [careful - don't merge yet] handlebars 3+4 Jun 21, 2016
@matteofigus matteofigus changed the title [careful - don't merge yet] handlebars 3+4 [careful] handlebars 3+4 Jun 30, 2016
@matteofigus
Copy link
Member Author

I think this is ready to be reviewed. This is pretty huge and kind of a big deal so I'd like a LGTM from multiple people, perhaps @antwhite @jankowiakmaria @mattiaerre

The behaviour here is:

  • Handlebars v4 is from here used by the CLI for compiling new components.
  • When component is already published with v3 compiler, the client-side client, the node.js client, and the registry all are able to still use v3 for the execution

I tried to add some tests there and there to ensure both versions are supported, but another pair of eyes is good to ensure all is good and covered.

@matteofigus matteofigus added this to the v1.0.0 milestone Jun 30, 2016
@antwhite antwhite merged commit 4b013a2 into master Jul 1, 2016
@matteofigus matteofigus deleted the handlebars-upgrade branch July 1, 2016 17:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants