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

Remove html brace definition as a default #107

Merged
merged 1 commit into from
Jan 16, 2019

Conversation

SergeyCherman
Copy link

Currently ng2-ace-editor imports html mode by default. If a user of the component doesn't need html mode it still pulls it in, inflating the bundle size. I suggest bumping the major version after this commit so that existing users aren't impacted.

@evgenius1424
Copy link

Nice fix. Any progress?

@SergeyCherman
Copy link
Author

@fxmontigny Anything I can do to get this merged?

@SergeyCherman
Copy link
Author

@fxmontigny Anyway to get this merged? I'd rather not create another fork for ng2 ace.

@fxmontigny
Copy link
Owner

Hi,
I can't build a new version because my build script are broken.
I need to find why 😢 .
I try tomorrow.

@SergeyCherman
Copy link
Author

Thanks! Let me know if it's something I can look at. Happy to help!

@fxmontigny fxmontigny merged commit 8386ba8 into fxmontigny:master Jan 16, 2019
@fxmontigny
Copy link
Owner

Thanks @SergeyCherman can you try my new branch https://github.com/fxmontigny/ng2-ace-editor/tree/features/angular_7 ? thx

@SergeyCherman
Copy link
Author

LGTM, tested it and no more Peer Deps warnings! I can't try running the project since when using a git branch in package.json it won't work since it doesn't pull in the peer deps, but it should be fine once published on npm.

Can you bump the version on this? I suggest bumping major version to not break it for anyone how's assuming minor/patch updates won't break their projects.

@fxmontigny
Copy link
Owner

Yes you right.This is more simple.

I have create a new version 0.3.9. Can you try it?

@SergeyCherman
Copy link
Author

Yes, this works! 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
Development

Successfully merging this pull request may close these issues.

3 participants