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

amp-list / bind issue with optimizer #51

Closed
bjalford opened this issue Jun 11, 2018 · 3 comments
Closed

amp-list / bind issue with optimizer #51

bjalford opened this issue Jun 11, 2018 · 3 comments

Comments

@bjalford
Copy link

We're seeing an issue with the bind 'on' being removed front end on the optimised version. The issue occurs inside amp-list on the optimised version. The lightbox works on the valid amp version

How to replicate: Scroll to comments section at bottom of articles and click 'reply' on a comment

Note some AMP bind items work e.g. the 'Subscribe' link will launch the lightbox.

AMP version: Reply

Inspect element on optimised version: Reply

Optimised amp: https://www.independent.co.uk/news/world/asia/trump-kim-meeting-live-updates-latest-time-date-location-stream-a8392686.html

Valid amp: https://www.independent.co.uk/news/world/asia/trump-kim-meeting-live-updates-latest-time-date-location-stream-a8392686.html?amp

@sebastianbenz
Copy link
Collaborator

This sounds like: ampproject/amphtml#15834. Did you enable the DevChannel when calculating the runtime version?

I don't think that this is an issue with AMP Optimizer, as the on attribute is still present in the template markup produced by the optimizer - the rest happens at runtime.

@bjalford
Copy link
Author

Thanks - agree that seems the likely cause.

No, we're using current version code below which generates this: https://cdn.ampproject.org/rtv/001528391646530/v0/amp-live-list-0.1.js

const AmpOptimizerMiddleware = require('amp-toolbox-optimizer-express');
const runtimeVersion = require('amp-toolbox-runtime-version');

const { corsEnabledUrl } = require('../config');

module.exports = function () {
  const optimiser = AmpOptimizerMiddleware.create({
    runtimeVersion: () => runtimeVersion.currentVersion()
  });

  return (req, res, next) => {
    const matched = corsEnabledUrl
      .concat(['/pwamp-preview'])
      .filter((path) => req.url.startsWith(path));

    if (matched.length > 0) {
      next();
      return;
    }
    optimiser(req, res, next)
  }
};

@sebastianbenz
Copy link
Collaborator

Closing this for now. Feel free to reopen in case it turns out that it's caused by Optimizer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants