teambit / envs

Component development environments for the Bit community
https://bit.dev/bit/envs
Other
63 stars 9 forks source link

compiler should not declare external modules #37

Closed KutnerUri closed 4 years ago

KutnerUri commented 4 years ago

When adding compiler react-typescript@3.1.7 to a component, it generates a d.ts file called _bit_types.d.ts_. This file contains the default types for .gif, .png, .css, etc:

declare module '*.bmp' {
    const src: string;
    export default src;
}

// declare ...

These declarations can only happen once. Commonly, typescript project also includes these declarataions.

React:

❯ ls -l src
total 88
    App.css
    App.test.tsx
    App.tsx
    index.css
    index.tsx
    logo.svg
    > react-app-env.d.ts
    serviceWorker.ts

Vue:

❯ ls -l src
total 32
  App.vue
  assets
  components
  main.ts
  router
  > shims-tsx.d.ts
  > shims-vue.d.ts
  views

So, in framework projects there tends to already be a declaration of global types. If we include these types again in components, declaration will happen twice and the user will get a conflict. Even two components with the d.ts will cause an error.

Possible solutions:

Reference:

bit version : 14.5.0 node version : v12.12.0 npm version : 6.11.3 yarn version : 1.19.1 platform : darwin

qballer commented 4 years ago

The best solution is running this as a peer dependency. This will give it one copy of the dependency and typescript will run it once. There are still several issues with that. I'm also exploring ideas with specific module names. If they can be relative it solves the problem. I'm not sure that is possible.

This is something I don't know the best solution for and I would love to have a discussion and on why specific path is best for UI components.

KutnerUri commented 4 years ago

I am worried that projects could have hidden declaration file (unless the user ejects). Therefore I believe an isolated solution is better.

Peer dependency will work, but should be added only when needed. (ie. for components with no css at all, it should not have dependency)

Consider using typed css modules, but if it is too complicated, { [className: string]: string } is good enough.

qballer commented 4 years ago

It seems like the best solution is creating a declaration file which looks like this:


declare const style: {
    classes: { readonly [key: string]: string };
}
export=style

Inside index.tsx you will have following import:

import style from './index.css'

The file name of the deceleration should be index.css.d.ts

still need to figure out what is the best content for style

qballer commented 4 years ago

In this solution since all d.ts files are relative to the module there will be no global pollution and hence no redefinition. As for typed-css-modules, we should use it, but it only covers one use case among many. The solution above will cover most use cases I think.

KutnerUri commented 4 years ago

Sounds good to me. ✓ isolated ✓ correct types ✓ fast x some values may be undefined (but that's common practice, as they are only 1 import away)

qballer commented 4 years ago

x some values may be undefined (but that's common practice, as they are only 1 import away)

Care to share more ?

KutnerUri commented 4 years ago

only case this fails is:

/* cat.modules.css */
.cat {
  font-weight: bold;
}
/* cat.tsx */
import types from './cat.modules.css';

export default () => (
  //types.cought is undefined! 
  <div className={types.cought}>
    meow
  </div>
)

this is of low severity

qballer commented 4 years ago

fixed by 3.1.14