chore(docs): use correct script-URL for plnkr on snapshot #15438
Conversation
| @@ -13,7 +13,7 @@ var cdnUrl = googleCdnUrl + versionInfo.cdnVersion; | |||
| // The currentVersion may not be available on the cdn (e.g. if built locally, or hasn't been pushed | |||
| // yet). This will lead to a 404, but this is preferable to loading a version with which the example | |||
| // might not work (possibly in subtle ways). | |||
| var examplesCdnUrl = versionInfo.isSnapshot ? | |||
| var examplesCdnUrl = versionInfo.currentVersion.isSnapshot ? | |||
| (angularCodeUrl + 'snapshot') : | |||
| (googleCdnUrl + (versionInfo.version || versionInfo.currentVersion)); | |||
There was a problem hiding this comment.
Isn't this also wrong then? (there is no versionInfo.version)
There was a problem hiding this comment.
You are right. I couldn't find where version may have been defined, so I left it untouched in order to avoid breaking anything. But it is clear now that (a) versionInfo can never have a version property and (b) it is referring to the version property of the SemVer object (returned from semver.parse(...)) of versionInfo.currentVersion.
There was a problem hiding this comment.
BTW, it doesn't matter in practice, because Semver.prototype.toString() just returns the version property, but I will change it to versionInfo.currentVersion.version just in case.
There was a problem hiding this comment.
I think you are right @Narretz
There was a problem hiding this comment.
@gkalpak That's right. Maybe we can simplify this later. For now, let's get the fix merged, so the examples start working again.
|
LGTM. We can handle the comment later (if it needs to be handled) |
What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
Bug fix (for docs app).
What is the current behavior? (You can also link to an open issue here)
The URLs used for scripts when opening an example from the snapshot version in plnkr is incorrect and the code does not work.
See #15437.
What is the new behavior (if this is a feature change)?
The correct script URLs are used and the examples work.
Does this PR introduce a breaking change?
No.
Please check if the PR fulfills these requirements
Tests for the changes have been added (for bug fixes / features)Docs have been added / updated (for bug fixes / features)Other information:
Fixes #15437