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.

Sign up for GitHub

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

Jump to bottom

Combobox displayValue regression in @headlessui/vue@v1.7.5 #2084

Closed
Libertie opened this issue Dec 10, 2022 · 2 comments · Fixed by #2087
Closed

Combobox displayValue regression in @headlessui/vue@v1.7.5 #2084

Libertie opened this issue Dec 10, 2022 · 2 comments · Fixed by #2087
Assignees
@RobinMalfait

Comments

@Libertie
Copy link

What package within Headless UI are you using?

@headlessui/vue

What version of that package are you using?

v1.7.5

What browser are you using?

Firefox

Reproduction URL

https://stackblitz.com/edit/vue-ls2gkt?file=src/components/Combo.vue

Describe your issue

The latest release breaks the use of displayValue for Combobox. The reproduction link above shows the example from https://headlessui.com/vue/combobox (converted to options API for stackblitz). This code runs in headlessui/vue 1.7.4 but fails in 1.7.5 with error person is undefined.
@RobinMalfait RobinMalfait self-assigned this Dec 12, 2022
@RobinMalfait RobinMalfait mentioned this issue Dec 12, 2022
Merged
RobinMalfait added a commit that referenced this issue Dec 12, 2022
@RobinMalfait
fix regression where displayValue crashes
42ea386 
It regressed in the sense that it now uses `displayValue` for the
`defaultValue` as well, but if nothing is passed it would crash.

Right now, it makes sure to only run the displayValue value on the
actual value and the actual default value if they are not undefined.

Note: if your displayValue is implemented like `(value) => value.name`,
and your `value` is passed as `null`, it will still crash (as expected)
because then you are in charge of rendering something else than null. If
we would "fix" this, then no value can be rendered instead of `null`.

Fixes: #2084
@RobinMalfait
Copy link
Collaborator

Hey! Thank you for your bug report!
Much appreciated! 🙏

The reason this regressed is because we now also use the displayValue for the defaultValue, but if you didn't provide one then it would crash. Bad oversight on our part, sorry about that.

We now have some tests that should prevent this from happening in the future.

This should be fixed by #2087, and will be available in the next release.

You can already try it using:

  • npm install @headlessui/react@insiders.
  • npm install @headlessui/vue@insiders.

@sgehrman
Copy link

sgehrman commented Dec 12, 2022 •
edited

Is this related? Building on NextJS fails with a ComboBox
I just updated headless and now it won't build.

Run `npm audit` for details.
info  - Linting and checking validity of types  
info  - Creating an optimized production build  
info  - Compiled successfully
info  - Collecting page data  
[    ] info  - Generating static pages (0/27)
Error occurred prerendering page "/sandbox". Read more: https://nextjs.org/docs/messages/prerender-error
TypeError: Cannot read properties of undefined (reading 'name')
    at displayValue (/home/steve/Documents/GitHub/mavrik/fe-infinity/build/server/chunks/2409.js:604:54)
    at Object.render (file:///home/steve/Documents/GitHub/mavrik/fe-infinity/node_modules/@headlessui/react/dist/components/combobox/combobox.js:1:9592)
    at a.b.render (/home/steve/Documents/GitHub/mavrik/fe-infinity/node_modules/react-dom/cjs/react-dom-server.node.production.min.js:43:194)
    at a.b.read (/home/steve/Documents/GitHub/mavrik/fe-infinity/node_modules/react-dom/cjs/react-dom-server.node.production.min.js:41:83)
    at Object.exports.renderToString (/home/steve/Documents/GitHub/mavrik/fe-infinity/node_modules/react-dom/cjs/react-dom-server.node.production.min.js:52:138)
    at Object.renderPage (/home/steve/Documents/GitHub/mavrik/fe-infinity/node_modules/next/dist/server/render.js:680:46)
    at Object.defaultGetInitialProps (/home/steve/Documents/GitHub/mavrik/fe-infinity/node_modules/next/dist/server/render.js:350:67)
    at Function.getInitialProps (/home/steve/Documents/GitHub/mavrik/fe-infinity/build/server/chunks/6859.js:28:20)
    at Object.<anonymous> (/home/steve/Documents/GitHub/mavrik/fe-infinity/node_modules/next/dist/shared/lib/utils.js:75:33)
    at Generator.next (<anonymous>)
info  - Generating static pages (27/27)

> Build error occurred
Error: Export encountered errors on following paths:
        /sandbox
    at /home/steve/Documents/GitHub/mavrik/fe-infinity/node_modules/next/dist/export/index.js:404:19
    at runMicrotasks (<anonymous>)
    at processTicksAndRejections (node:internal/process/task_queues:96:5)
    at async Span.traceAsyncFn (/home/steve/Documents/GitHub/mavrik/fe-infinity/node_modules/next/dist/trace/trace.js:79:20)
    at async /home/steve/Documents/GitHub/mavrik/fe-infinity/node_modules/next/dist/build/index.js:1229:21
    at async Span.traceAsyncFn (/home/steve/Documents/GitHub/mavrik/fe-infinity/node_modules/next/dist/trace/trace.js:79:20)
    at async /home/steve/Documents/GitHub/mavrik/fe-infinity/node_modules/next/dist/build/index.js:1091:17
    at async Span.traceAsyncFn (/home/steve/Documents/GitHub/mavrik/fe-infinity/node_modules/next/dist/trace/trace.js:79:20)
    at async Object.build [as default] (/home/steve/Documents/GitHub/mavrik/fe-infinity/node_modules/next/dist/build/index.js:65:29)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@RobinMalfait RobinMalfait

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix regression where displayValue crashes tailwindlabs/headlessui
3 participants
@sgehrman @Libertie @RobinMalfait