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

Add autoprefixer to documentation page CSS #3812

Merged
merged 1 commit into from
Mar 6, 2019

Conversation

Munter
Copy link
Contributor

@Munter Munter commented Mar 6, 2019

This PR replaces #3803

@Munter Munter requested a review from plroebuck March 6, 2019 00:22
@coveralls
Copy link

Coverage Status

Coverage remained the same at 91.684% when pulling c957f8c on Munter/docs-autoprefixer into f1662ac on master.

@plroebuck
Copy link
Contributor

plroebuck commented Mar 6, 2019

Not following how AG-B automagically runs "autoprefixer" for us using "/docs/.browserlistrc",
as "autoprefixer" is not an AG-B dependency.

Based on your reply, see why you might not desire to make it a direct dependency.
Maybe add "autoprefixer" as AG-B optional dependency?

@Munter
Copy link
Contributor Author

Munter commented Mar 6, 2019

We had some of these bring-your-own-version module dependencies marked as optional at one point in time. The problem is how the default npm behavior shows that to the user at install time. First off, an optional dependency is installed at install time, but is allowed to fail while still succeeding the root level installation run. However, a failed optional install is exactly as verbose with red blinking lights as a failed installation of a dev or non-dev dependency, causing every user to assume the installation failed despite the exit code being 0.

Secondarily, in order to actually control the specific version you'd like from an optional dependency, we'd need to specify the wides possible version range, basically *, and you'd still have to save the version of your own choice as a direct dependency of your project.

So we concluded that optionalDependency is bollocks in its current form and went over to make a runtime check for module availability and have assetgraph log an info event that explains how you can install a specific optional module to gain certain improvements in your build. https://github.com/assetgraph/assetgraph/blob/master/lib/transforms/autoprefixer.js#L17-L28

We're actively working on reducing some of these more optional parts of the pipeline. I have a branch running where we've eliminated the need for two-thirds of these types of checks because the node ecosystem now provides modules that wrap certain OS-level binaries we were depending on to do the same work before. But the concept of letting developers chose their own version of specific libraries still remains

@boneskull boneskull merged commit b27bc60 into master Mar 6, 2019
@boneskull boneskull added type: chore generally involving deps, tooling, configuration, etc. semver-patch implementation requires increase of "patch" version number; "bug fixes" labels Mar 6, 2019
@boneskull boneskull added this to the v6.1.0 milestone Mar 6, 2019
@Munter Munter deleted the Munter/docs-autoprefixer branch March 7, 2019 21:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver-patch implementation requires increase of "patch" version number; "bug fixes" type: chore generally involving deps, tooling, configuration, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants