Fixed peer dependencies issue of download page #1491
Conversation
download.js
Outdated
| @@ -136,13 +136,15 @@ for (var category in components) { | |||
|
|
|||
| var filepath = all.meta.path.replace(/\{id}/g, id); | |||
|
|
|||
| var after = all[id].peerDependencies || all[id].after; | |||
There was a problem hiding this comment.
I'd have to dig through the download page logic a bit, but should we be combining peerDeps + after in some way?
There was a problem hiding this comment.
@mAAdhaTTah They are separated now.
There was a problem hiding this comment.
My initial thought was something more like:
var after = all[id].after;
if (all[id].peerDependencies) {
after.push($u.type(all[id].peerDependencies) === 'string' ? [all[id].peerDependencies] : all[id].peerDependencies);
}which would be a little simpler but would need a couple extra type checks.
Thoughts?
There was a problem hiding this comment.
It's kind of unnecessary because there is always only either after (plugins) or peerDependencies (languages).
download.js
Outdated
| function getSortedComponentsByRequirements(components){ | ||
| var sorted = getSortedComponents(components, "after"); | ||
| function getSortedComponentsByRequirements(components, secondaryOrderKey) { | ||
| var sorted = getSortedComponents(components, secondaryOrderKey); |
There was a problem hiding this comment.
@mAAdhaTTah Do you have a better idea for the name of this variable?
There was a problem hiding this comment.
It's called requireName in getSortedComponents; might as well go with that.
There was a problem hiding this comment.
Well, that's ambiguous because "require" is also a literal requireName.
Maybe optionalRequireName ?
There was a problem hiding this comment.
I just noticed that the name optional requirement is really bitter sweet.
I'll just use afterName.
|
Is there anything else that has to be done? |
The details are discussed here #1490.