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
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
18 changes: 18 additions & 0 deletions packages/upward-js/lib/resolvers/ComputedResolver.js
@@ -0,0 +1,18 @@
const debug = require('debug')('upward-js:ComputedResolver');
const AbstractResolver = require('./AbstractResolver');

class ComputedResolver extends AbstractResolver {
static get resolverType() {
return 'computed';
}
static get telltale() {
return 'computed';
}
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.

}
}

module.exports = ComputedResolver;
@@ -0,0 +1,14 @@
const ComputedResolver = require('../ComputedResolver');

test('resolverType is computed', () =>
expect(ComputedResolver.resolverType).toBe('computed'));

test('telltale exists', () => expect(ComputedResolver.telltale).toBeDefined());

test('returns empty string', () => {
const visitor = {
upward: jest.fn()
};
const resolver = new ComputedResolver(visitor);
expect(resolver.resolve()).resolves.toBe('');
});
3 changes: 2 additions & 1 deletion packages/upward-js/lib/resolvers/__tests__/index.test.js
Expand Up @@ -16,6 +16,7 @@ test('Resolvers exports', () => {
proxy: expect.any(Function),
service: expect.any(Function),
template: expect.any(Function),
url: expect.any(Function)
url: expect.any(Function),
computed: expect.any(Function)
});
});
3 changes: 2 additions & 1 deletion packages/upward-js/lib/resolvers/index.js
Expand Up @@ -7,7 +7,8 @@ const ResolverList = [
require('./ProxyResolver'),
require('./DirectoryResolver'),
require('./ConditionalResolver'),
require('./UrlResolver')
require('./UrlResolver'),
require('./ComputedResolver')
];

const ResolversByType = ResolverList.reduce((out, Resolver) => {
Expand Down
36 changes: 36 additions & 0 deletions packages/upward-spec/README.md
Expand Up @@ -57,6 +57,10 @@ See [UPWARD_MAGENTO.md](UPWARD_MAGENTO.md) for context on how UPWARD fills a nee
- [UrlResolver Example](#urlresolver-example)
- [UrlResolver Configuration Options](#urlresolver-configuration-options)
- [UrlResolver Notes](#urlresolver-notes)
- [ComputedResolver](#computedresolver)
- [ComputedResolver Example](#computedresolver-example)
- [ComputedResolver Configuration Options](#computedresolver-configuration-options)
- [ComputedResolver Notes](#computedresolver-notes)
- [Reducing boilerplate](#reducing-boilerplate)
- [Default parameters](#default-parameters)
- [Builtin constants](#builtin-constants)
Expand Down Expand Up @@ -505,6 +509,7 @@ A Resolver is an object which describes how a value is obtained. There are five
- `ProxyResolver` delegates request/response handling to a proxy
- `DirectoryResolver` delegates request/response handling to a static file directory
- `UrlResolver` builds a URL from strings and other URLs
- `ComputedResolver` resolves to a PHP class to get data

Each Resolver takes different configuration parameters. Like a context lookup string, a resolver represents an operation which will execute and then deliver its results upward in the tree, until all dependencies of the top-level `status`, `headers`, and `body` definitions are resolved.

Expand Down Expand Up @@ -1086,6 +1091,37 @@ https://admin.host:8081/api/rest/v1/adminToken?refreshToken=a1b2c3&role=owner

- If both `search` and `query` are present, the parameters must be merged, giving preference to `query` where there are conflicts. _"Array" query parameters are not defined by this specification, since their behavior is inconsistent across platforms._

### ComputedResolver

The ComputedResolver is used for the UPWARD PHP implementation to inline key page data in the initial page response without needing to make external or
round trip calls. This essentially provides a way to add more resolvers than are available in this spec. **It can be noted that this resolver breaks spec
and its JS implementation is only to avoid crashing during build.**

#### ComputedResolver Example

```yaml
someDataPoint:
resolver: computed
type:
resolver: inline
inline: typeKey
```

The ComputedResolver's resolved value can vary depending on the class used. The typeKey maps to a PHP class, who takes responsibility for returning a value.

#### ComputedResolver Configuration Options

| Property | Type | Default | Description |
| ---------- | --------------------------- | -------- | -----------------------------------------------------------------------------------------------------------|
| `type` | `Resolved<string>` | | _Required_. This value is used to map to a PHP class that will return a value. |
| `<other>` | `Resolved<any>` | | Since this is essentially a way to add more resolvers, any data defined in the yaml is passed to the class |

#### ComputedResolver Notes
In its PHP implementation, the definition and resolver is passed to the class, so any information the class needs can be passed in this manner. The type is mapped
to a PHP class through the di.xml and the class has access to any Magento dependency injection it needs.
The JS implementation is only present to not break when the resolver type is defined.


## Reducing boilerplate

The above examples are fairly verbose, to make the workings of UPWARD configuration especially clear. The UPWARD specification is intentionally verbose in its canonical format, to enable maximal static analysis. However, An UPWARD-compliant server must also include features to reduce boilerplate.
Expand Down
59 changes: 19 additions & 40 deletions packages/venia-ui/upward.yml
Expand Up @@ -105,58 +105,37 @@ veniaPageType:
resolver: inline
inline:
data:
resolver: conditional
when:
- matches: env.SCRIPT_NAME
pattern: '.*\.php$'
use:
resolver: computed
type:
resolver: inline
inline: pageType
additional:
- type: product
fetch: '__typename,id'
- type: cms_page
fetch: 'identifier'
- type: category
fetch: 'id'
default:
inline: ''
resolver: computed
type:
resolver: inline
inline: pageType
additional:
- type: product
fetch: '__typename,id'
- type: cms_page
fetch: 'identifier'
- type: category
fetch: 'id'

# Nonce for page type inline script
veniaPageTypeNonce:
resolver: inline
inline:
nonce:
resolver: conditional
when:
- matches: env.SCRIPT_NAME
pattern: '.*\.php$'
use:
resolver: computed
type:
resolver: inline
inline: pageTypeNonce
default:
inline: ''
resolver: computed
type:
resolver: inline
inline: pageTypeNonce

# Webpack chunks to preload on page based on page type
veniaWebpackChunks:
resolver: inline
inline:
scripts:
resolver: conditional
when:
- matches: env.SCRIPT_NAME
pattern: '.*\.php$'
use:
resolver: computed
type:
resolver: inline
inline: webpackChunks
default:
inline: ''
resolver: computed
type:
resolver: inline
inline: webpackChunks

# The veniaAppShell object resolves to a response that returns server-side
# rendered HTML containing the PWA application shell.
Expand Down