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

Issues with our type definitions #195

Closed
benoitgrelard opened this issue Sep 29, 2020 · 8 comments · Fixed by #196
Closed

Issues with our type definitions #195

benoitgrelard opened this issue Sep 29, 2020 · 8 comments · Fixed by #196
Labels
Type: Bug Confirmed bug

Comments

@benoitgrelard
Copy link
Collaborator

benoitgrelard commented Sep 29, 2020

A tester reported top issue in this sandbox: https://codesandbox.io/s/stitches-stuff-d0m44?file=/src/Dialog.tsx

After digging, it seems to be an issue around the types coming off of our modules.
As you can see things are coming out as any.

I did a bit more digging locally (in a local CRA) and found a few very strange things:

  • on codesandbox we get this:
    image
  • local CRA doesn't complain like that but there are still some issues
  • Dialog and parts still come in as any
  • styles come kinda typed ok, but digging into the built types reveal some weird stuff:
    image
    image
  • it's missing a few part names (trigger, close), and has old ones (inner). I have no idea where it's getting this from… I have done a fresh build and get this too 🤔
  • the Dialog and parts coming as any I think is due to this:
    image
  • This looks like it didn't resolve properly when building the type definitions, not sure why… but it seems that's what is invalidating the whole type.
@benoitgrelard benoitgrelard added the Type: Bug Confirmed bug label Sep 29, 2020
@benoitgrelard
Copy link
Collaborator Author

@chaance @jjenzz just pinging here for now before assigning anyone.

@benoitgrelard
Copy link
Collaborator Author

benoitgrelard commented Sep 29, 2020

Also not sure if it's related or not, but trying to build a single package gives me this:

image

@jjenzz
Copy link
Contributor

jjenzz commented Sep 29, 2020

Also not sure if it's related or not, but trying to build a single package gives me this:

Have you tried yarn workspace @interop-ui/react-dialog build from root?

@benoitgrelard
Copy link
Collaborator Author

Also not sure if it's related or not, but trying to build a single package gives me this:

Have you tried yarn workspace @interop-ui/react-dialog build from root?

Same error exactly when doing this from root.

@benoitgrelard
Copy link
Collaborator Author

I guess it makes sense, because utils hasn't been built yet. That's why we need the whole topological order thing right?

@benoitgrelard
Copy link
Collaborator Author

benoitgrelard commented Sep 29, 2020

Ok a bit more on this:

  • I did a new build locally
  • still getting the old parts in styles.
  • I then deleted my local .parcel-cache folder
  • I ran another build
  • now the parts are correct in styles

🤔 That's kinda worrying I guess, it means something was using the cache rather than latest stuff.
@jjenzz I seem to remember there was some discussion around caching in parcel, do you remember if that was related?

That's one of the issues potentially resolved (although I'm not sure that's a real solution).
I'm still trying to figure out how to deal with the other one.

@jjenzz
Copy link
Contributor

jjenzz commented Sep 30, 2020

That's interesting. I ran yarn workspace @interop-ui/react-dialog build locally and it worked hence the suggestion so I wonder how my parcel-cache wasn't in a tizzy and yours was 🤔 definitely not good. Perhaps we should suffix our publish build script with --no-cache for now.

What's the remaining issue? The types? I wonder if the polymorphic stuff I was doing would help with all this 😬

As a quick test, we could try deleting the DialogStaticProps stuff and doing the Object.assign thing instead. That will reduce one layer of complexity in our types and might help reveal something 🤷‍♀️

@benoitgrelard
Copy link
Collaborator Author

benoitgrelard commented Sep 30, 2020

That's interesting. I ran yarn workspace @interop-ui/react-dialog build locally and it worked hence the suggestion so I wonder how my parcel-cache wasn't in a tizzy and yours was 🤔 definitely not good. Perhaps we should suffix our publish build script with --no-cache for now.

Well, that's still wrong right? It worked for you probably because you had done a full build at some point and had the @interop-ui/utils package build in its dist. So when building just @interop-ui/react-dialog, it did find @interop-ui/utils/dist/index.js. If you clean your cache before you'll see you'll get the same error.

What's the remaining issue? The types? I wonder if the polymorphic stuff I was doing would help with all this 😬
As a quick test, we could try deleting the DialogStaticProps stuff and doing the Object.assign thing instead. That will reduce one layer of complexity in our types and might help reveal something 🤷‍♀️

I don't think it has anything to do with this.
If you look at that screenshot I posted in the original issue:
image

See the import("react/utils/src").ForwardRefExoticComponentWithAs part?

It's more as if it's not resolving properly when building the type definitions.
Perhaps something to do with the monorepo?
I don't know if that import() thing should work, but regardless it definitely shouldn't be pointing to src right?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Bug Confirmed bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants