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

Handle the lack of the optional dependencies #3791

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

mems
Copy link

@mems mems commented Apr 4, 2023

What:

When optional dependencies are not installed or missing (ex: npm ci --omit=optional) less.js throw errors for features that use that dependencies.

This is in contradiction with the purpose of optionaldependencies:

It is still your program's responsibility to handle the lack of the dependency.

Why:

This is a graceful degradation when optional dependencies are not installed or missing. Less.js will still generate CSS but with a less optimal result. (ex: data-uri() always generate application/octet-stream base64 URIs, image-size() resolve to 0px 0px)

How:

Checklist:

  • Documentation N/A
  • Added/updated unit tests N/A (how can we can emulate the absence of a module ?)
  • Code complete

@matthew-dean
Copy link
Member

matthew-dean commented Apr 4, 2023

EDIT: 🤔 Okay I thought about this for another bit. If someone is explicitly using image-size, and whatever optional dependency is not installed, then Less definitely should throw an error, although the error should be explicit about what they need to do (e.g. install the dependency). Outputting no error or warning is IMO worse DX.

In this case, I would take "It is still your program's responsibility to handle the lack of the dependency" as it is the responsibility of Less to throw an error that is instructive, for each of these errors. It should not silently insert values that are other than the stylesheet author's intent. That is, when they use image-size(), they expect to get the size of the image as an output. A zero-dimension result is not the size of the image.

Copy link
Member

@matthew-dean matthew-dean left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See comment

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

Successfully merging this pull request may close these issues.

None yet

2 participants