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

useToken 'breakpoints' not returning values #5045

Closed
1 of 3 tasks
IHIutch opened this issue Nov 9, 2021 · 11 comments · Fixed by #5117
Closed
1 of 3 tasks

useToken 'breakpoints' not returning values #5045

IHIutch opened this issue Nov 9, 2021 · 11 comments · Fixed by #5117
Labels
Breaking Change 🔨 Changes that break compatibility with previous versions of Chakra UI. Type: Bug 🐛 Something isn't working

Comments

@IHIutch
Copy link

IHIutch commented Nov 9, 2021

Description

When I use 'breakpoints' in useToken, I expect it to return a width value

Link to Reproduction

https://codesandbox.io/s/flamboyant-poincare-19xm1

Steps to reproduce

const [sm, lg] = useToken("breakpoints", ["sm", "lg"]);
const [red500] = useToken("colors", ["red.500"]);

console.log({ sm, lg, red500 });

Chakra UI Version

1.6.12

Browser

Chrome Version 95.0.4638.69 (Official Build) (x86_64)

Operating System

  • macOS
  • Windows
  • Linux

Additional Information

Looks like this broke in this release #4867 (system@1.7.5), because its still working in #4799 (system@1.7.4)

Looking a little deeper, it looks like its caused by this change because breakpoints is not a part of __cssMap

@IHIutch IHIutch added needs triage Issues and pull requests that need triage attention Type: Bug 🐛 Something isn't working labels Nov 9, 2021
@IHIutch
Copy link
Author

IHIutch commented Nov 9, 2021

Without knowing a better way to solve this issue it seems like rolling back to before #4842 and finding a better solution makes the most sense because 4842 is a breaking change.

Using __cssMap decidedly doesn't do what useToken says it does, which is using theme as the parent key. eg: theme.colors.red.100

@IHIutch
Copy link
Author

IHIutch commented Nov 10, 2021

Thinking about this a bit more, I'm wondering if something like stitches provides some guidance here.

I'm also thinking that if you avoided dot notation, you could use brackets for explicitly identifying nested parameters. For example:

const lg = useToken("breakpoints", "lg");
const red500 = useToken("colors", "red[500]");
const space = useToken("spaces", "1.5");

@primos63
Copy link
Contributor

Your sandbox is not going to work as you have the useToken calls before the ChakraProvider is invoked. The theme is not available at the time you're making the call so it always returns the default value which in this case the values you requested.
This is how the app should be written

import React from "react";
import ReactDOM from "react-dom";
import { Box, ChakraProvider, useToken, Text } from "@chakra-ui/react";
import { get } from "@chakra-ui/utils"
import { useTheme } from "@emotion/react";

function App() {
  const theme = useTheme()
  const [sm, lg] = ['sm', 'lg'].map(bkpt => get(theme, `breakpoints.${bkpt}`, bkpt));
  const [red500] = useToken("colors", ["red.500"]);

  return (
    <Box padding={4}>
      <Text>{sm}</Text>
      <Text>{lg}</Text>
      <Text>{red500}</Text>
    </Box>
  );
}

const rootElement = document.getElementById("root");
ReactDOM.render(
  <ChakraProvider>
    <App />
  </ChakraProvider>,
  rootElement
);

It will produce the output

30em
62em
#E53E3E

The breakPoints are not in the __cssMap. I think this is because they are used by media queries so they don't need to be css variables.

@IHIutch
Copy link
Author

IHIutch commented Nov 10, 2021

Your sandbox is not going to work as you have the useToken calls before the ChakraProvider

Ah yes, how foolish of me! That is now updated.

@IHIutch
Copy link
Author

IHIutch commented Nov 10, 2021

Sorry, why is this closed? The updated CodeSandbox shows the bug.

@anubra266
Copy link
Collaborator

Sorry, why is this closed? The updated CodeSandbox shows the bug.

@IHIutch refer to @primos63' answer above, this is not a bug, breakpoints are not exposed as CSS variables

@IHIutch
Copy link
Author

IHIutch commented Nov 10, 2021

I understand what is causing the issue, I identified that myself in my initial comment.

The useToken hook as defined is

The useToken hook retrieves whatever is in the theme at the given path(s).

This is no longer true and causes unexpected behavior. There is no mention of a tokens relationship with CSS variables.

It's also an undocumented breaking change for anyone using breakpoints with useToken, which doesn't throw an error.

@anubra266 anubra266 reopened this Nov 10, 2021
@anubra266 anubra266 added Breaking Change 🔨 Changes that break compatibility with previous versions of Chakra UI. and removed needs triage Issues and pull requests that need triage attention labels Nov 10, 2021
@anubra266
Copy link
Collaborator

cc @segunadebayo

@primos63
Copy link
Contributor

@IHIutch Other than the CSB mistake, I was not pointing out causation. I provided what I thought was the reasoning for the omission of breakpoints. I do not know the reasoning as I did not implement the change. I did not close the issue for that reason.

If the omission was intentional, then I agree it would be a breaking change that the documentation should have reflected. However, I believe it's simply an unintentional omission that will require either adding breakpoints to the __cssMap or additional code similar to what I provided earlier.

Appreciate your time and effort on this.

@IHIutch
Copy link
Author

IHIutch commented Nov 10, 2021

@primos63 Yeah, no worries. I appreciate your comment, I was hoping accurately capture the new change (and in my case, breaking change).

@tomchentw
Copy link
Contributor

I've upgraded from 1.6.10 to 1.7.0 and experienced the same issue. Thanks for raising this up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Breaking Change 🔨 Changes that break compatibility with previous versions of Chakra UI. Type: Bug 🐛 Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants