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

PWA-2355 UPWARD JS resolver for Computed type #3533

Merged

Conversation

justinconabree
Copy link
Contributor

Description

To bring the JS implementation of the UPWARD spec closer to the PHP implementation, we've added the Computed resolver to it as well. The resolver simply returns an empty string so the application can continue without error.

Related Issue

Closes PWA-2355

Acceptance

Verification Stakeholders

@zetlen

Specification

Changes to upward-spec and/or upward-js packages must be reviewed

Verification Steps

Test scenario(s) for direct fix/feature

  • Run DEBUG=upward-js:ComputedResolver yarn stage:venia
  • Visit front-end
  • Should see upward-js:ComputedResolver Computed resolver is meant for UPWARD PHP only in the terminal
  • Site should function normally, and inlined data should be missing from source (see screenshot for reference)
    image

Test scenario(s) for any existing impacted features/areas

  • Test using UPWARD PHP
  • Inlined data should be available (Can look for global variable INLINED_PAGE_TYPE in browser console)

Checklist

  • I have added tests to cover my changes, if necessary.
  • I have added translations for new strings, if necessary.
  • I have updated the documentation accordingly, if necessary.

@m2-community-project m2-community-project bot added this to Ready for Review in Pull Request Progress Nov 3, 2021
@pwa-studio-bot pwa-studio-bot added pkg:upward-js Pertains to upward-js reference implementation of UPWARD. pkg:upward-spec Pertains to UPWARD specification package. pkg:venia-ui labels Nov 3, 2021
@pwa-studio-bot
Copy link
Collaborator

pwa-studio-bot commented Nov 3, 2021

Fails
🚫 A version label is required. A maintainer must add one.
Messages
📖

Associated JIRA tickets: PWA-2355.

📖 DangerCI Failures related to missing labels/description/linked issues/etc will persist until the next push or next nightly build run (assuming they are fixed).
📖

Access a deployed version of this PR here. Make sure to wait for the "pwa-pull-request-deploy" job to complete.

Generated by 🚫 dangerJS against 1e9abe5

Copy link
Contributor

@mikhaelbois mikhaelbois left a comment

Choose a reason for hiding this comment

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

LGTM ✅

async resolve() {
debug('Computed resolver is meant for UPWARD PHP only.');

return '';
Copy link
Contributor

Choose a reason for hiding this comment

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

Wouldn't we be able to at least supply webpageChunks based on the page type similar to how the php implementation globs for those files?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@fooman Not without the page type information unfortunately. Each root component is its own chunk (sometimes more than one), and we only inline the chunks for the current page type.

@m2-community-project m2-community-project bot moved this from Reviewer Approved to Review in Progress in Pull Request Progress Nov 6, 2021
@fooman fooman removed their assignment Nov 6, 2021
@michaelyu0123 michaelyu0123 added the version: Patch This changeset includes backwards compatible bug fixes. label Nov 8, 2021
@michaelyu0123 michaelyu0123 merged commit 603d1c5 into develop Nov 8, 2021
@m2-community-project m2-community-project bot moved this from Review in Progress to Done in Pull Request Progress Nov 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Partner: Absolunet Inc partners-contribution pkg:upward-js Pertains to upward-js reference implementation of UPWARD. pkg:upward-spec Pertains to UPWARD specification package. pkg:venia-ui Progress: done version: Patch This changeset includes backwards compatible bug fixes.
Development

Successfully merging this pull request may close these issues.

None yet

6 participants