Skip to content
This repository was archived by the owner on Mar 5, 2025. It is now read-only.

4x webpack build config #5111

Merged
merged 15 commits into from
Jun 16, 2022
Merged

4x webpack build config #5111

merged 15 commits into from
Jun 16, 2022

Conversation

jdevcs
Copy link
Contributor

@jdevcs jdevcs commented Jun 7, 2022

Description

#5068

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist:

  • I have selected the correct base branch.
  • I have performed a self-review of my own code.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have made corresponding changes to the documentation.
  • My changes generate no new warnings.
  • Any dependent changes have been merged and published in downstream modules.
  • I ran npm run dtslint with success and extended the tests and types if necessary.
  • I ran npm run test:cov and my test cases cover all the lines and branches of the added code.
  • I ran npm run build with success.
  • I have tested the built dist/web3.min.js in a browser.
  • I have tested my code on the live network.
  • I have checked the Deploy Preview and it looks correct.
  • I have updated the CHANGELOG.md file in the root folder.

@jdevcs jdevcs added the 4.x 4.0 related label Jun 7, 2022
@jdevcs jdevcs self-assigned this Jun 7, 2022
@jdevcs jdevcs marked this pull request as ready for review June 7, 2022 13:36
@jdevcs jdevcs requested a review from nazarhussain June 7, 2022 14:52
@jdevcs jdevcs requested review from nazarhussain and luu-alex June 13, 2022 16:27
Copy link
Contributor

@nazarhussain nazarhussain left a comment

Choose a reason for hiding this comment

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

We need to observe these points explicitly.

  1. We are not building whole repository with webpack
  2. We are not also not building every package in the mono repo with the webpack
  3. We only need web3 package to be build with the webpack.

So we have to be very explicit to create the configuration and build steps for particular packages which we need.

  1. Create a symlink of webpackprod.config.js under packages/web3/ to follow the consistent pattern we have for other configuration files.
  2. Create a task postbuild under the packages/web3/package.json to execute the webpack.
  3. Create a directory packages/web3/dist_web for the output of webpack.
  4. To keep the compatibility with 1.x create a symlink dist on root level which points to packages/web3/dist_web

I will highly recommend to not commit the build files in the version control. That does not mean we will be breaking anything for users. We can build and keep the dist folders when we publish the packages, so it will not be any issue for any user. Users who would be using local clones of the source code they would never bee using prebuild files anyway.

None of the above points will introduce any breaking changes for users. The way we are managing builds could be different on our end, which is fine as we have differences of many workflows compared to 1.x and 4.x.

@avkos
Copy link
Contributor

avkos commented Jun 14, 2022

We need to observe these points explicitly.

  1. We are not building whole repository with webpack
  2. We are not also not building every package in the mono repo with the webpack
  3. We only need web3 package to be build with the webpack.

So we have to be very explicit to create the configuration and build steps for particular packages which we need.

  1. Create a symlink of webpackprod.config.js under packages/web3/ to follow the consistent pattern we have for other configuration files.
  2. Create a task postbuild under the packages/web3/package.json to execute the webpack.
  3. Create a directory packages/web3/dist_web for the output of webpack.
  4. To keep the compatibility with 1.x create a symlink dist on root level which points to packages/web3/dist_web

I will highly recommend to not commit the build files in the version control. That does not mean we will be breaking anything for users. We can build and keep the dist folders when we publish the packages, so it will not be any issue for any user. Users who would be using local clones of the source code they would never bee using prebuild files anyway.

None of the above points will introduce any breaking changes for users. The way we are managing builds could be different on our end, which is fine as we have differences of many workflows compared to 1.x and 4.x.

I also agree with the text above. it makes sense to go this way.

@jdevcs jdevcs requested a review from nazarhussain June 15, 2022 16:01
@jdevcs
Copy link
Contributor Author

jdevcs commented Jun 15, 2022

We need to observe these points explicitly.

  1. We are not building whole repository with webpack
  2. We are not also not building every package in the mono repo with the webpack
  3. We only need web3 package to be build with the webpack.

So we have to be very explicit to create the configuration and build steps for particular packages which we need.

  1. Create a symlink of webpackprod.config.js under packages/web3/ to follow the consistent pattern we have for other configuration files.
  2. Create a task postbuild under the packages/web3/package.json to execute the webpack.
  3. Create a directory packages/web3/dist_web for the output of webpack.
  4. To keep the compatibility with 1.x create a symlink dist on root level which points to packages/web3/dist_web

I will highly recommend to not commit the build files in the version control. That does not mean we will be breaking anything for users. We can build and keep the dist folders when we publish the packages, so it will not be any issue for any user. Users who would be using local clones of the source code they would never bee using prebuild files anyway.

None of the above points will introduce any breaking changes for users. The way we are managing builds could be different on our end, which is fine as we have differences of many workflows compared to 1.x and 4.x.

Thanks for your feedback. As discussed in call we can make these changes in future if there is any limitation or requirement for keeping webpack script at web3 package level.

@nazarhussain
Copy link
Contributor

@jdevcs I was thinking on our discussion lately. During the ETHPrague when I introduced new schema based eth compatible validator to the community. It was one discussion point and I told them they can use these features in their projects independently of the web3. With that perspective it's required to build few other packages for the web as well. As it's few lines of change, so we can make it part of the alpha release as well so community can use those.

Different web based package managers e.g. bower tend to utilize the browser field int he package.json and we can point this field only from within the package. Without that field user have to always remember and point the distribution files themselves.

All these points aligned to the direction of building packages independently with webpack (for the packages we tend to think useable).

@nazarhussain
Copy link
Contributor

I created a small PR to reflect what I am describing here. Let's see what other team members think about it.

Copy link
Contributor

@avkos avkos left a comment

Choose a reason for hiding this comment

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

need to fix one test or maybe just relaunch it

@nazarhussain nazarhussain dismissed their stale review June 16, 2022 12:12

As explained in very details already, I don't think the direction of this PR is appropriate to proceed.

@jdevcs jdevcs merged commit 4ee3cf3 into 4.x Jun 16, 2022
@jdevcs jdevcs deleted the junaid/4xwebpackbuildconfig branch June 16, 2022 13:19
@jdevcs
Copy link
Contributor Author

jdevcs commented Jun 16, 2022

As explained in very details already, I don't think the direction of this PR is appropriate to proceed.

@nazarhussain As we discussed in call these are not urgent requirements in alpha for individual builds of other packages but instead some other items are urgent ( doc, breaking changes fixes, and some other PRs ) so when there is requirement we can implement enhancements after going through our sprint process and team discussion.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
4.x 4.0 related
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants