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

Change import style of 'ajv' and 'lodash' in all non test files #1242

Merged
merged 2 commits into from
Jan 24, 2019

Conversation

sdirix
Copy link
Member

@sdirix sdirix commented Jan 24, 2019

No description provided.

@edgarmueller edgarmueller merged commit a80c05d into eclipsesource:master Jan 24, 2019
@coveralls
Copy link

Coverage Status

Coverage increased (+0.2%) to 88.365% when pulling 01647ff on sdirix:fix-imports into db68697 on eclipsesource:master.

@edgarmueller edgarmueller added this to the 2.2.0 milestone Jan 25, 2019
@zummed
Copy link

zummed commented Jan 28, 2019

I had to add:

"allowSyntheticDefaultImports": true

to my tsconfig.json after trying out version "2.1.1-2.1.1-beta.0.0" just now, presumably because of this change. Without it the following error is generated:

/data/work/analytics/tegu/node_modules/@jsonforms/core/lib/actions/index.d.ts
(1,8): Module '"/data/work/analytics/tegu/node_modules/@jsonforms/core/node_modules/ajv/lib/ajv"' has no default export.

@sdirix
Copy link
Member Author

sdirix commented Jan 28, 2019

Hi @zummed. You are right, we'll update our README.MD for that. While "allowSyntheticDefaultImports": true allows you to compile, you might run into runtime issues (could you test that?). You might need to set "esModuleInterop": true.

@edgarmueller
Copy link
Contributor

edgarmueller commented Jan 28, 2019

Stefan is right, and I'm sorry about the broken version identifier. I have already released a new version with the correct one, which is 2.1.1-beta.0.

@zummed
Copy link

zummed commented Jan 28, 2019

Ah thank you both :-) Yes no probs, I'll continue with the beta version so will keep an eye out for weirdness.

However I'm currently on react-redux 6.x, so aware some faults might be my doing.

@edgarmueller
Copy link
Contributor

@zummed Usage of react-redux 6.x as a peer dependency should be fine with the latest changes (redux 4.x is not though, see #1244). Do you experience any issues?

@zummed
Copy link

zummed commented Jan 28, 2019

@edgarmueller That's good to know thanks. I am using redux 4.x (4.0.1) so the same wariness applies :-)

Up until today I've had no issues (using react and the material renderer) so all good! However, I started using an array in the schema today which is failing, hence my upgrade. Not looked into the cause yet, but looks like it's my fault with the redux version from the initial error output.

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.

4 participants