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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Default import doesn't return a component after codebase was migrated to Typescript #434

Closed
tricoder42 opened this issue Jan 20, 2020 · 7 comments
Labels
Status: Unconfirmed Bug reports without a repro, not yet confirmed

Comments

@tricoder42
Copy link

tricoder42 commented Jan 20, 2020

馃悰 Bug report

Current Behavior

VisuallyHidden component causes error, because default import returns an object instead of a component:

import VisuallyHidden from '@reach/visually-hidden'

console.log(VisuallyHidden)
// {
//   default: {
//     ... component here
//   }
// }

Expected behavior

Default import should return a component:

import VisuallyHidden from '@reach/visually-hidden'

console.log(VisuallyHidden)
// {
//   ... component here
// }

Reproducible example

Fresh Next.js install with only Typescsript and @reach/visually-hidden dependencies.
GitHub Repository

Suggested solution(s)

When I edit source of visually-hidden.development.js manually and change last line exports.default = to module.exports = , then it works. It's either build issue or my tsconfig issue.

Edit: It seems that build files are missing Object.defineProperty(exports, '__esModule', { value: true }); line. Add this line to source solves the problem as well. I've checked tsdx, and it seems to be configured correctly (i.e. respecting esModuleInterop settings from tsconfig.json)

Additional context

  • It only affects components with default exports only (i.e. VisuallyHidden). Other components works fine (e.g. Menu).
  • I tried update tsconfig, but I'm using the official one from Next.js without any modification.
  • The bug appeard after 0.7.0, when codebase was migrated to Typescript.
  • I tried to interop default export manually and noticed, this only happens on server (i.e. running in Node.js).

Your environment

Software Name(s) Version
Reach Package @reach/visually-hidden 0.7.3
Typescript 3.7.5
@tricoder42
Copy link
Author

This bug seems to be caused by upstream library, tsdx, which ignores custom tsconfig when configuring rollup, see jaredpalmer/tsdx#436

As a workaround, we could rename all tsconfig.build.json in package directories to default tsconfig.json. I'll send a PR.

@chaance
Copy link
Member

chaance commented Jan 22, 2020

Putting this on my plate for tomorrow morning. Thanks for raising the flag!

@chaance chaance added the Status: Unconfirmed Bug reports without a repro, not yet confirmed label Jan 22, 2020
@tricoder42
Copy link
Author

I tried to simply rename all tsconfig.build.json to tsconfig.json, but I got a bunch of different errors. I didn't have much time to investigate it further. Folks at tsdx already approved my PR, so the issue could be fixed in upstream soon.

Meanwhile, I'm converting imports manually:

import _VisuallyHidden from "@reach/visually-hidden"

export function interopDefault(obj) {
  return obj.default ? obj.default : obj
}

const VisuallyHidden = interopDefault(_VisuallyHidden)

AFAIK it only affects VisuallyHidden and Portal components. All other packages use named exports.

@stefanmaric
Copy link

AFAIK it only affects VisuallyHidden and Portal components. All other packages use named exports.

Just came here because of the broken Popover component export. But I guess that one doesn't count since it doesn't seem to be public. 馃槃

@danieltott
Copy link
Contributor

I had to use the workaround to get @reach/alert working in a test as well.

chaance added a commit that referenced this issue Jan 24, 2020
@chaance
Copy link
Member

chaance commented Jan 25, 2020

I patched the tsdx issue for the moment, so please try upgrading to 0.7.4 and let me know if anyone still has issues. Seems to be working fine on my end.

@chaance chaance closed this as completed Jan 25, 2020
@tricoder42
Copy link
Author

I can confirm the issue is solved in 0.7.4. Thanks @chancestrickland

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Unconfirmed Bug reports without a repro, not yet confirmed
Projects
None yet
Development

No branches or pull requests

4 participants