core: `background` doesn't want to show up (without any errors)

[dzearing]

In the playground, I tried adding this:

import { makeStyles, shorthands } from '@griffel/core';

export default makeStyles({
  root: {
    backgroundColor: 'red',
  },
  foo: {
    background: 'green'
  }
});

Expected: green shows up in the output.

Resulted:

.f3xbvq9 {
  background-color: red;
}

Changing background to backgroundColor fixes it. But this was the first thing I tried, and it failed to update or even show an error message. Would be good to expand shorthand.

[spmonahan]

Shorthands like background are not supported. #30 provides more details on this.

The shorthands import in the playground provides helper functions for writing shorthands but it does not appear to support background at the moment.

[dzearing]

I think it's fine to do that for performance and the css variable nondeterministic reasons, but I think there needs to be feedback to train developers to do the right thing; ideally in the IDE like lint rules which can add red squiggle lines under incorrect syntax with fixers to adjust. In the sandbox, there was no error. It just didn't work, leaving curious people frustrated and moving on to other solutions.

The ideal would be to "just work" and manage this at runtime, but address performance costs by optimizing at babel transform time. I think that's possible for expansion; just not for the padding: --var(--padding) scenario where the caller is providing shorthand values intended for shorthand properties. Maybe that could be avoided by providing helpers around defining variables. (Again some kind of linting, or even runtime name checks that get stripped in production.)

[dzearing]

Another thought... are there APIs browsers could add here to simplify some of this? (E.g. shorthand parser api or something.) We should consider if there are asks to browser vendors if that could make this easier.

[layershifter]

I think it's fine to do that for performance and the css variable nondeterministic reasons, but I think there needs to be feedback to train developers to do the right thing; ideally in the IDE like lint rules which can add red squiggle lines under incorrect syntax with fixers to adjust. In the sandbox, there was no error. It just didn't work, leaving curious people frustrated and moving on to other solutions.

It's totally valid feedback, appreciate it 👍

• it already logs errors to console
• we have a PR to improve try out experience, #79. Improve error reporting in sandbox is also one of the goals
• we have TypeScript checks, so it will be 🟥
• ESLint plugin is coming (https://github.com/microsoft/griffel/tree/main/packages/eslint-plugin) to improve that experience and provide suggestions

Do you have ideas on how to improve it more?

[dzearing]

I've been thinking about it. I think the most ideal would be to do something possibly expensive at runtime, but removable at build time, like:

if (name === 'padding') {
  expandedProps = padding(value)
}

...basically, call the helper to auto expand for the user so they don't need to apply extra mental cycles on something they've used forever.

If you detect the value is a css variable, maybe you could do something special there:

// given this: { padding: '--var(--somePaddingShorthand)' } // 4px 4px 3px

if (name === 'padding') {
  if (isVariable(value)) {
    console.warn(`Using css variables for shorthand values has edge case issues and is discouraged. Use variables for atomic css properties only.`);
  }

Then at transform time, expand things and remove the runtime overhead.

Also consider removing the helpers themselves in retail/transformed output.

I think even if there are ts/lint/console errors about the usage, it's going to create mental headaches for the dev experience. It's a fine stopgap but the ideal is "it just works"

[layershifter]

@dzearing CSS shorthands are supported by the latest version of Griffel, #531 🎉