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

Update angular package to use Ivy #269

Merged
merged 2 commits into from
Mar 7, 2023
Merged

Update angular package to use Ivy #269

merged 2 commits into from
Mar 7, 2023

Conversation

Theaninova
Copy link
Contributor

@Theaninova Theaninova commented Jan 29, 2023

Fixes #249
Should fix #256, fix #251, fix #250

  • Update to Angular 15
  • Replaced TSLint with ESLint + Prettier
  • Updated various dependencies

Update to Angular 15
Replace TSLint with ESLint + Prettier

Update dependencies
@Theaninova
Copy link
Contributor Author

As a temporary workaround until a new release is made, you can copy this file into your project and replace the @ionic/storage-angular with @ionic/storage

@dnietodev
Copy link

dnietodev commented Feb 2, 2023

Hello, im highly interested in this PR to be merged, when can we expect to have a new version with this fix?
Thanks!

@mlynch
Copy link
Contributor

mlynch commented Mar 7, 2023

Thanks for the PR. How does this impact people on older versions of Angular? I'm not up to speed on the latest module format. I'm assuming it'll require a major version release?

@Theaninova
Copy link
Contributor Author

Thanks for the PR. How does this impact people on older versions of Angular? I'm not up to speed on the latest module format. I'm assuming it'll require a major version release?

Good question, I don't know actually, but I think it will not work with Angular <9 or anyone who has chosen to opt-out of Ivy.

I think it's worth mentioning that with Angular 9 (first version with Ivy by default) being released in early 2020, it has been out of LTS for 1.5 years already, and view engine has been deprecated since v12 which is also out of LTS.

@mlynch mlynch merged commit 229b5e7 into ionic-team:main Mar 7, 2023
@mlynch
Copy link
Contributor

mlynch commented Mar 7, 2023

Okay published @ionic/storage-angular 4.0.0-next.0@next if someone wants to test that and see how it works

@dtarnawsky
Copy link

If you test with https://github.com/ionic-team/ionifits by installing this version:
npm install @ionic/[email protected]

You get build errors:
Error: src/app/services/storage.service.ts:5:25 - error TS2307: Cannot find module '@ionic/storage-angular' or its corresponding type declarations.

@mlynch
Copy link
Contributor

mlynch commented Mar 8, 2023

Okay @ionic/[email protected]@next seems to be working locally for me. Will wait for more testers before cutting a 4.0 release

@WolfWalter
Copy link

WolfWalter commented Mar 17, 2023

Hi,
I get the same error as @dtarnawsky with@ionic/[email protected]@next.

./src/app/shared/services/persistent-storage.service.ts:2:0-49 - Error: Module not found: Error: Can't resolve '@ionic/storage-angular' in 'xxx/app/src/app/shared/services'

./src/main.ts:28:0-60 - Error: Module not found: Error: Can't resolve '@ionic/storage-angular' in 'xxx/app/src'

Error: src/app/shared/services/persistent-storage.service.ts:2:25 - error TS2307: Cannot find module '@ionic/storage-angular' or its corresponding type declarations.

2 import { Storage } from '@ionic/storage-angular';
                          ~~~~~~~~~~~~~~~~~~~~~~~~


Error: src/main.ts:28:36 - error TS2307: Cannot find module '@ionic/storage-angular' or its corresponding type declarations.

28 import { IonicStorageModule } from '@ionic/storage-angular';

@magyargergo
Copy link

magyargergo commented Mar 20, 2023

Hi, ich get the same error as @dtarnawsky with@ionic/[email protected]@next.

./src/app/shared/services/persistent-storage.service.ts:2:0-49 - Error: Module not found: Error: Can't resolve '@ionic/storage-angular' in 'xxx/app/src/app/shared/services'

./src/main.ts:28:0-60 - Error: Module not found: Error: Can't resolve '@ionic/storage-angular' in 'xxx/app/src'

Error: src/app/shared/services/persistent-storage.service.ts:2:25 - error TS2307: Cannot find module '@ionic/storage-angular' or its corresponding type declarations.

2 import { Storage } from '@ionic/storage-angular';
                          ~~~~~~~~~~~~~~~~~~~~~~~~


Error: src/main.ts:28:36 - error TS2307: Cannot find module '@ionic/storage-angular' or its corresponding type declarations.

28 import { IonicStorageModule } from '@ionic/storage-angular';

I tried to upgrade to @ionic/[email protected]@next as well and the respective node_modules folder was not having source files.

What else do we need to do to release 4.0? cc @mlynch

@Theaninova
Copy link
Contributor Author

Theaninova commented Mar 20, 2023

Hmm, I'll take a look today at what went wrong

--- edit ---

Okay so after a quick look at the changes I think what happened is that the new package format doesn't automatically re-exports Storage from @ionic/storage. This is of course intentional behavior from typescript. Now the question of course is, do we simply add a re-export in the file, or would we rather want to require users to also add @ionic/storage as a dependency? I could see arguments for both, but a re-export is probably the more sensible option since I'm not sure how angular would treat two different versions, it may be the case there that it doesn't treat them as the same injection token anymore and so wouldn't even inject it anyways.

TL;DR The fix is
index.ts

...

export * from "@ionic/storage"

--- edit 2 ---

Actually hold on this is a different issue, I'll take a look once I'm home

@boradakash
Copy link

boradakash commented Apr 10, 2023

Any update on this?
I also tried to upgrade to @ionic/storage-angular@4.0.0-next.1@next and the node_modules folder is not having source files.
and it returns following error:
image

Is there any workaround for this?

mlynch added a commit that referenced this pull request Apr 10, 2023
@mlynch
Copy link
Contributor

mlynch commented Apr 10, 2023

Okay I've fixed the package and republished it and can verify the dist files are correctly bundled, and I'm still getting this error now, so I'm going to revert this PR until we can figure out what's not working. I'm not experienced enough in Angular to do the work on this so if someone else was able to get this working end-to-end we can take another look at it, thanks.

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