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

chore(deps): upgrading lib to angular version 12 #771

Merged
merged 9 commits into from
May 31, 2021

Conversation

sapnildessai
Copy link
Contributor

#769 : Angular 12 support

@ghiscoding
Copy link
Owner

There seems to be lot of issues with the Jest unit tests, I'm not sure if it's because I didn't update Angular-Jest in a while but there's definitely something wrong here.

Test Suites: 31 failed, 135 passed, 166 total
Tests:       738 failed, 3 skipped, 2248 passed, 2989 total

@ghiscoding ghiscoding changed the title Upgrading angular version 12 WIP - chore(deps): upgrading lib to angular version 12 May 26, 2021
@sapnildessai
Copy link
Contributor Author

sapnildessai commented May 26, 2021

This is strange. When I run npm test on my local system it works pretty fine. I will take a look
image

tsconfig.json Outdated
@@ -12,7 +12,6 @@
"noUnusedParameters": false,
"noUnusedLocals": false,
"noImplicitReturns": true,
"emitDecoratorMetadata": true,
Copy link
Owner

@ghiscoding ghiscoding May 26, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this line got removed and might be causing the Jest unit tests to fail on CircleCI, the error also seem to show it uses some minified code unless I'm not reading it correctly

image

@ghiscoding
Copy link
Owner

@sapnildessai can you put back that line that I've identified in the last comment and see if that helps.

Update tsconfig with "emitDecoratorMetadata": true property
@sapnildessai
Copy link
Contributor Author

Yes. Added it back

@ghiscoding
Copy link
Owner

different error now, can you remove the flatpickr from these 2 Filters component since as it says it will always be true anyway. I'll soon rewrite them anyway to JavaScript native element instead of jQuery

image

Removed flatpickr variable since it is always defined
Removed flatpickr variable since it's always defined
@codecov
Copy link

codecov bot commented May 26, 2021

Codecov Report

Merging #771 (b1f1252) into master (2462c1b) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##            master      #771   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          161       161           
  Lines        11280     11280           
  Branches      4022      4022           
=========================================
  Hits         11280     11280           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2462c1b...b1f1252. Read the comment docs.

@ghiscoding
Copy link
Owner

now we're in business, it all works now 😃

Was that line removal "emitDecoratorMetadata": true produced by running ng update?
What might be better is to move that flag into the tsconfig.spec.json, which is the one used by Jest, and still remove it from the main tsconfig.json

@sapnildessai
Copy link
Contributor Author

Yay!! Yes it was added by ng update. Yes, let's remove it from main tsconfig and add it to spec json file and see.

Removing "emitDecoratorMetadata": true
Added "emitDecoratorMetadata": true property
@ghiscoding
Copy link
Owner

that doesn't seem to work, so let's put it back to when it passed and I'll look at fixing it in future PR (unless you want to troubleshoot it). Thanks a lot for the great contribution here 👍

@ghiscoding
Copy link
Owner

@sapnildessai hey so I don't think you replied to question, I would just like to confirm... have you also tried the Excel Export (which was that issue related to)?

@sapnildessai
Copy link
Contributor Author

Hey. Sorry which issue are you taking about? I haven't tried excel export after the upgrade.

@ghiscoding
Copy link
Owner

ghiscoding commented May 26, 2021

the issue #769, that other user wrote this

I have upgraded my application to angular12. Am using server side slick grid of 2.29.1 . But Slick grid is working fine except export to excel option..

@sapnildessai
Copy link
Contributor Author

No I haven't tried. Let me forcefully upgrade demo project and try it locally.

@sapnildessai
Copy link
Contributor Author

I have verified that excel export is working fine after upgrading to angular 12. I did a force update on the demo fork.

@ghiscoding ghiscoding changed the title WIP - chore(deps): upgrading lib to angular version 12 chore(deps): upgrading lib to angular version 12 May 31, 2021
@ghiscoding ghiscoding merged commit ecc3966 into ghiscoding:master May 31, 2021
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

Successfully merging this pull request may close these issues.

2 participants