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

feat: improve the build using rollup #882

Open
wants to merge 39 commits into
base: main
Choose a base branch
from

Conversation

mato533
Copy link
Contributor

@mato533 mato533 commented Dec 21, 2024

Description

To avoid complications of ESM and CJS build, Implement rollup as bundler.

We created new package @wdio/electron-bundler because there are some logic to create rollup configurations.
This could be achieved by simply creating a common configuration file and use from each package, but we packaged it to improve the quality of the entire library by introducing a unit test for the logic that creates the rollup configuration itself.

Core motivation

we have to set aligned value between output.dir of the rollup option and out Dir of the typescript option.

Specifically, the path of typescript compiler option 'outdir' must be located inside rollup 'dir' option.

To create a library that supports both CJS and ESM, the source code for each format needs to be stored in separate directories.(when not use cjs,mjs) This means different paths must be specified for output.dir in the Rollup configuration.

So, new package is created to genelate the rollup programmatically.
*) related info

Feature

  1. The input parameter is automatically created based on exports field of package,json
    Reduce the effort of synchronizing package.json settings and Rollup settings.

  2. Create parameters that enable ESM and CJS style output.

  3. Synchronizes @rollup-plugin-typescript settings with rollup settings and enables output of *.d.ts files.

Related Issue

#880

@goosewobbler
Since this is a major modification, I would appreciate it if you could give me your frank opinions on whether or not to create a new package, and on the following structural changes for dist.

@mato533 mato533 marked this pull request as draft December 23, 2024 16:33
@mato533 mato533 marked this pull request as ready for review December 23, 2024 17:01
@goosewobbler
Copy link
Member

I think extracting the rollup configs and having another package is ok, I would call it @wdio/electron-bundler - however, there's a lot of code here simply to create rollup config files. I think a simpler approach would be better, I will take a look at it in more depth over the holiday...

@mato533
Copy link
Contributor Author

mato533 commented Dec 24, 2024

@goosewobbler
thank you for your review.
your thoughts that is too much source code is understandable for me....

The part what increases the amount of source code is following functionality.

  • Determination for input option of rollup based on read exports field of package.json (related code)

  • Determination for output option of rollup based on read main , module field of package.json

    • If it will be determined that maintained manually or fixed value, as a result we can reduce the code. (related code)
  • some option parameter is existing that is not required for now (related code)

    • it has been removed. but plugin options are remains because it can only be set at generation time.

@mato533 mato533 marked this pull request as draft December 24, 2024 19:29
@mato533 mato533 marked this pull request as ready for review December 25, 2024 03:20
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