Report to Raven when a component does not support the requested brand. #188
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| // Therefore projects may have included it without setting the brand. | ||
| let oLayoutException = false; | ||
| if (allComponents.hasOwnProperty('o-layout')) { | ||
| oLayoutException = parseFloat(allComponents['o-layout'].version) < 3; |
There was a problem hiding this comment.
Would parseFloat work with a SemVer version such as 2.2.2?
There was a problem hiding this comment.
It would to confirm it's less than 3.
|
Is it possible to have some tests to cover this feature? |
@JakeChampion My intention is to add an integration test if we keep it (I'll create an issue with details). |
JakeChampion
approved these changes
Jan 22, 2019
notlee
added a commit
that referenced
this pull request
Mar 14, 2019
* Error when building a component for an unsupported brand. This is more helpful than broken CSS or a general Sass build failure. Relates to: #188
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
goal
SCSS compilation fails for
o-layout@v3.0.0-beta-1and the master brand. We don't intent to fix it, as o-layout only supports the internal brand.Instead of a random SCSS compilation error:
OBS should throw a more helpful error:
gotchas
o-colorshave added support for different brands at different times. It's possible that a branded component has been used for an unsupported brand.approach
Throwing an error when a component does not support the requested brand is technically a breaking due to gotcha 3. I think it's a reasonable error to throw though if we can confirm it won't break our users builds. This PR will report to Sentry any builds which would fail. If none are reported, we may assume we can safely throw an error for real (I suggest at least a month, to allow the max-age of builds to expire; a month will also allow for sites with little traffic to make a request, as branding is quite recent this concerns me less). 👈 Feedback on whether this is a dreadful idea appreciated.