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

Types clash when using along with @ionic/core@4.2.0 (again again 馃檮馃檮) #19

Closed
simonhaenisch opened this issue Apr 10, 2019 · 11 comments

Comments

@simonhaenisch
Copy link
Contributor

simonhaenisch commented Apr 10, 2019

To replace #18.

@mlynch @mhartington this is getting quite frustrating to work with... every time an Ionic interface changes, the @ionic/pwa-elements package is blocking us from updating ionic/core in our project.

[ ERROR ]  TypeScript: node_modules/@ionic/pwa-elements/node_modules/@ionic/core/dist/types/components.d.ts:5142:4
           Subsequent property declarations must have the same type. Property ''IonRange'' must be of type 'IonRange',
           but here has type 'IonRange'.

   L5141:  'IonRadio': Components.IonRadio;
   L5142:  'IonRange': Components.IonRange;
   L5143:  'IonRefresherContent': Components.IonRefresherContent;

[ ERROR ]  TypeScript: node_modules/@ionic/pwa-elements/node_modules/@ionic/core/dist/types/components.d.ts:5208:4
           Subsequent property declarations must have the same type. Property ''ion-img'' must be of type
           'IonImgAttributes', but here has type 'IonImgAttributes'.

   L5207:  'ion-header': Components.IonHeaderAttributes;
   L5208:  'ion-img': Components.IonImgAttributes;
   L5209:  'ion-infinite-scroll-content': Components.IonInfiniteScrollContentAttributes;

[ ERROR ]  TypeScript: node_modules/@ionic/pwa-elements/node_modules/@ionic/core/dist/types/components.d.ts:5242:4
           Subsequent property declarations must have the same type. Property ''ion-range'' must be of type
           'IonRangeAttributes', but here has type 'IonRangeAttributes'.

   L5241:  'ion-radio': Components.IonRadioAttributes;
   L5242:  'ion-range': Components.IonRangeAttributes;
   L5243:  'ion-refresher-content': Components.IonRefresherContentAttributes;

May I suggest removing ionic from the dependencies (there's only one interface imported anyway), and adding

{
  "peerDependencies": {
    "@ionic/core": "^4.0.0"
  }
}

to package.json?

@MCanhisares
Copy link

Do we have an update on this?

@jase88
Copy link

jase88 commented Apr 24, 2019

Workaround for us was to use selective dependency resolutions with yarn

package.json
"resolutions": { "@ionic/core": "4.3.0" },

@corysmc
Copy link

corysmc commented May 3, 2019

Ran into this issue today as well.

@simonhaenisch
Copy link
Contributor Author

simonhaenisch commented May 5, 2019

Just found a solution to get around this... you can add a path mapping for @ionic/core to tsconfig.json so that it resolves @ionic/core to the same path for both your project and @ionic/pwa-elements:

{
  "compilerOptions": {
    // ...

    "baseUrl": ".", // needed to make "paths" work
    "paths": {
      "@ionic/core": ["node_modules/@ionic/core"]
    }
  }
}

I have a suggestion for a better and cleaner solution though, that would be a bit of a breaking change but solve this long-term: make Ionic an external dependency. That would mean that when including @ionic/pwa-elements as a script, @ionic/core would have to be included first. When using it in a project, @ionic/core should be a peer dependency. Because of how Ionic components are lazy-loaded, I think this approach would work fine. If agreed, I can try and make a PR for that (/cc @mlynch @mhartington)?

@corysmc
Copy link

corysmc commented May 6, 2019

Thanks @simonhaenisch that fixes it for me!

@mlynch
Copy link
Contributor

mlynch commented May 13, 2019

Does changing this to use peerDependencies resolve it?

@mlynch
Copy link
Contributor

mlynch commented May 13, 2019

Hey all, we're working on some changes that should permanently fix this. Stay tuned.

@simonhaenisch
Copy link
Contributor Author

Just saw that you removed Ionic completely... that's another way of making it work 馃槃 I liked the idea of it being an extension of Ionic (i. e. it would be able to use ion-toast), however there doesn't seem to be a simple way of maintaining that.

@jcesarmobile
Copy link
Member

as 1.1.0 doesn't use @ionic/core, this can be closed

@SoftronetBithin
Copy link

Just found a solution to get around this... you can add a path mapping for @ionic/core to tsconfig.json so that it resolves @ionic/core to the same path for both your project and @ionic/pwa-elements:

{
  "compilerOptions": {
    // ...

    "baseUrl": ".", // needed to make "paths" work
    "paths": {
      "@ionic/core": ["node_modules/@ionic/core"]
    }
  }
}

I have a suggestion for a better and cleaner solution though, that would be a bit of a breaking change but solve this long-term: make Ionic an external dependency. That would mean that when including @ionic/pwa-elements as a script, @ionic/core would have to be included first. When using it in a project, @ionic/core should be a peer dependency. Because of how Ionic components are lazy-loaded, I think this approach would work fine. If agreed, I can try and make a PR for that (/cc @mlynch @mhartington)?

Thanks @simonhaenisch . It works for me.

@mlynch
Copy link
Contributor

mlynch commented May 19, 2019

@simonhaenisch we do want to get back to that point, and it should work. Just given everyone's time it wasn't going to work before we hit 1.0 so we needed to punt on that. There's some compiler work still to be done first.

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

No branches or pull requests

7 participants