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

Extension point should come after processing other members #629

Closed
mgiuca opened this issue Nov 27, 2017 · 1 comment
Closed

Extension point should come after processing other members #629

mgiuca opened this issue Nov 27, 2017 · 1 comment

Comments

@mgiuca
Copy link
Collaborator

mgiuca commented Nov 27, 2017

The steps for processing a manifest defines an extension point that allows proprietary and externally specified members to be inserted and have post-processing steps defined.

However, it is Step 3 of the algorithm, occurring before any of the standard manifest members are post-processed, which means any custom logic is unable to make use of the final value of other manifest fields, such as scope. I think it should come last (i.e., between current Step 14 and 15).

For example, I am currently working on a share_target extension shown here: [preview] [pull request]

The post-processing step for share_target requires the final version of scope. If I were to define share_target in the manifest spec itself, it would have its post-processing step performed somewhere between Step 7 and 15. Since I am defining it in an external spec, I need to add it at the extension point, which is before scope's post-processing steps. I guess I could run the post-processing the scope member algorithm redundantly from my spec, but that seems unwieldy.

Is there any harm in moving the extension point to the end of this algorithm?

@marcoscaceres
Copy link
Member

I'm ok with us moving it... the idea was to prevent people overriding standard behavior. If that becomes a problem, we can modify this to be:

const defaultValues = processManifest();
const extendedValues = processExtensions(defaultValues);
const finalValue = {...extendedValues, ...defaultValues};
return finalValue;

mgiuca added a commit to mgiuca/manifest that referenced this issue Feb 9, 2018
Now appears at the end, which allows extensions to make use of the
processed values.

Closes w3c#629.
mgiuca added a commit that referenced this issue Feb 9, 2018
Now appears at the end, which allows extensions to make use of the
processed values.

Closes #629.
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