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

Fix type-imports according to the release notes of TypeScript 3.8 #1845

Merged
merged 1 commit into from
Dec 22, 2021

Conversation

mirismaili
Copy link
Contributor

@sdirix
Copy link
Member

sdirix commented Dec 6, 2021

Hi @mirismaili, thanks for the PR. Can you tell about the motivation? Do you run into problems when not refactoring those specific type imports/exports?

@mirismaili
Copy link
Contributor Author

mirismaili commented Dec 6, 2021

Hi @sdirix

Your welcome.

I am trying to add some unsupported features to the library (additionalProperties and patternProperties / #1765) for our business. So I created a dev-project that I can test my changes into the library, quickly (using hot-reload).

My different webpack/typescript configuration (at the dev-project) caused some issues. I fixed them (there) to match JSONForms. But this special case was an issue in JSONForms. So I decided to fix it here.

@mirismaili
Copy link
Contributor Author

I think the below configuration can be changed after this fix (be applied to all packages):

loaders: [ 'babel-loader', 'ts-loader' ], // first ts-loader, then babel-loader

But I'm not sure.

@sdirix
Copy link
Member

sdirix commented Dec 9, 2021

@mirismaili The PR conflicts now, can you rebase on latest master?

@mirismaili
Copy link
Contributor Author

@sdirix
Rebased.

@coveralls
Copy link

coveralls commented Dec 10, 2021

Coverage Status

Coverage decreased (-0.1%) to 85.251% when pulling 779e05e on mirismaili:type-import-export into 81f1f19 on eclipsesource:master.

@mirismaili mirismaili force-pushed the type-import-export branch 2 times, most recently from e1bebab to 8fab66b Compare December 18, 2021 16:29
Copy link
Member

@sdirix sdirix left a comment

Choose a reason for hiding this comment

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

LGTM!

@sdirix sdirix merged commit 6d8b63e into eclipsesource:master Dec 22, 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.

3 participants