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

Adding webpack 4 in the build process, upgrading to React 16.3.1 and removing support for IE 9 and 10 #815

Merged
merged 89 commits into from
May 3, 2018

Conversation

matuzalemsteles
Copy link
Member

@matuzalemsteles matuzalemsteles commented Apr 13, 2018

New

  • New build process with Webpack 4
  • Update React to v16.3.1
  • Use of the new refs API
  • Install transform-class-properties for static use in class

Changes

  • Refactor in codebase bdbd6b0
  • Corrections of the tests
  • Replace React.addons.TestUtils with react-dom/test-utils
  • Removes the use of PropTypes

Breakings

  • Our components are no longer exposed as before by accessing the AlloyEditor.ButtonBold will cause undefined for example. We continue with AlloyEditor.Buttons[...key].
  • Removing support for IE 9 and 10

Notes

Components

Made minor changes to some components, see ff816fe, f66fda1, 3d7633e...

Tests

Many of the tests have been set to work again with the new build process, some changes consist of imports of files or mocks of functions.

Skip some test

Ref: 4ecd00c

It was necessary to pass some tests since the UIBRIDGE components have not yet been refactored and are having problems with findAllInRenderedTree and isCompositeComponentWithType.

Use gulp for generate files

For a moment we are maintaining the use of gulp to generate the css files, icon fonts, copy files from CKEDITOR, languages and generate the demo. In the near future we will be able to remove the gulp by scripts next to the Webpack.

Insights

CI

Test run times averaged 4 min compared to 7 min. 🙂

Estimate

Bundle

With the new build process with webpack 4, the size of some of our packages have remained the same and others have increased because of things the webpack ends up adding to create the wrapper or adding polyfill if necessary.

New structure

  • dist/ - Build Files
  • lib/ - CKEditor Files
  • scripts/ - Build-related scripts and utilities for testing.
  • src/ - All code base, Core, Plugins and UI
  • test/ - All of our tests together
  • package.json - npm module manifest

Future

Our structure still needs refinements, but we need to work on our code base to continue polishing architecture over time.

  • Using Flow in code base 🎉🙂
  • Remove the use of gulp
  • Remove use of create-react-class
  • Remove use of the lifecycle APIs from the React componentWillMount, componentWillReceiveProps and componentWillUpdate.

jbalsas and others added 30 commits February 14, 2018 08:39
- Simplifies build step (webpack)
- Removes UI layers and keeps only react
- Updates to react 16
It will be absolute to use the gulp in the next changes.
TODO: We are altering the direct basePath of the CKEDITOR object so that it works with the final bundle, we still have to analyze if it is the ideal one.
@matuzalemsteles matuzalemsteles changed the title [DO NOT MERGE] Some tests... Adding webpack 4 in the build process, upgrading to React 16.3.1 and removing support for IE 9 and 10 Apr 20, 2018
@jbalsas
Copy link
Contributor

jbalsas commented May 3, 2018

This looks like an awesome starting point, @matuzalemsteles!

I'm merging this in so we can continue on top of it.

There are some concerns regarding CKEditor License change, so we might need to stick with 4.x for our 2.x version...

@jbalsas jbalsas changed the base branch from master to develop May 3, 2018 12:09
@jbalsas jbalsas merged commit e6627c1 into liferay:develop May 3, 2018
@jbalsas
Copy link
Contributor

jbalsas commented May 3, 2018

I've merged this into develop and forked 1.x branch for future maintenance of the existing version

Great job @matuzalemsteles !!

jbalsas added a commit that referenced this pull request Aug 21, 2018
This reverts commit e6627c1, reversing
changes made to 8287772.
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