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

refactor: migrate configtest package to ESM #3439

Closed
wants to merge 46 commits into from
Closed

Conversation

snitin315
Copy link
Member

@snitin315 snitin315 commented Oct 8, 2022

What kind of change does this PR introduce?
refactor.

Did you add tests for your changes?
No
If relevant, did you update the documentation?
No
Summary
Start ESM migration.

Does this PR introduce a breaking change?
No

Other information
No

@snitin315 snitin315 requested a review from a team as a code owner October 8, 2022 10:32
Copy link
Member

@evenstensberg evenstensberg left a comment

Choose a reason for hiding this comment

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

Maybe draft these ESM converted packages and group them up, until every package is done migrated?

@snitin315
Copy link
Member Author

Yes, I'll convert this to a draft.

@snitin315 snitin315 marked this pull request as draft October 8, 2022 11:23
@alexander-akait
Copy link
Member

@snitin315 I think in this PR we need to convert all to ESM (or alternative we can use import(...)) to load extensions for webpack-cli, so they will work

rishabh3112 and others added 19 commits November 6, 2022 14:53
BREAKING CHANGE: the `migrate` command was removed without replacement, please use migration guide
…3290)

BRAKING CHANGE:  the `--hot` option was removed for `webpack build` and `webpack watch` commands in favor directly usage `HotModuleReplacement` plugin
BREAKING CHANGE: the `--prefetch` option was removed without replacement
* fix(webpack-cli): add an option for preventing interpret

* fix: define the option for built-in flags

* docs: add descriptions of the option

* refactor: rename `--config-registered` to `--disable-interpret`

* fix: change conditional statement

* refactor: standalone test

* test: use `--disable-interpret` without transpilation

* docs: fix the description

Co-authored-by: Anshuman Verma <[email protected]>

* refactor: built-in options type

Co-authored-by: Nitin Kumar <[email protected]>

* test: re-update snapshots

* fix: add double quote

Co-authored-by: Nitin Kumar <[email protected]>

* test: update snapshots for webpack4

* chore: remove `--require` from `test:coverage`

* test: update snapshots

Co-authored-by: Anshuman Verma <[email protected]>
Co-authored-by: Nitin Kumar <[email protected]>
BREAKING CHANGE: webpack-cli no longer supports webpack v4, the minimum supported version is webpack v5.0.0
…0.0` (#3353)

* refactor!: the minimun supported webpack-dev-server version in v4.0.0

* refactor: remove unused type
@codecov
Copy link

codecov bot commented Nov 6, 2022

Codecov Report

Merging #3439 (f2eb1eb) into next (f2eb1eb) will not change coverage.
The diff coverage is n/a.

❗ Current head f2eb1eb differs from pull request most recent head e0737fe. Consider uploading reports for the commit e0737fe to get more accurate results

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             next    #3439   +/-   ##
=======================================
  Coverage   84.82%   84.82%           
=======================================
  Files          22       22           
  Lines        1588     1588           
  Branches      447      447           
=======================================
  Hits         1347     1347           
  Misses        241      241           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f2eb1eb...e0737fe. Read the comment docs.

@snitin315
Copy link
Member Author

@alexander-akait I suppose if we move to ESM it will break compatibility with webpack v5 too. Because webpack 5 is using cli with require

Screenshot 2022-11-06 at 4 05 28 PM

@alexander-akait
Copy link
Member

alexander-akait commented Nov 7, 2022

@snitin315 What about if we will use import(...)? It should work

@snitin315
Copy link
Member Author

@alexander-akait yes but it means from webpack v5.0.0 to the current latest version of webpack will not work with this major version. It will be compatible only with future version of webpack which is using import().

@alexander-akait
Copy link
Member

Hm, I cannot undestand... Webpack can have any type - commonjs or ESM, we just load it using import(...) and reuse, if you want to use ESM package in commonjs - just use import(...)... it is the official working solution

@snitin315
Copy link
Member Author

@alexander-akait Yes import(...) will work. But it means users will have to update webpack to the latest version which includes this PR webpack/webpack#16448

@alexander-akait
Copy link
Member

@snitin315 Oh, I got it, yeah, let's postpone it then, and mark it for webpack v6, we will need to do it for webpack-dev-server too

@snitin315
Copy link
Member Author

@alexander-akait yes, let's postpone.

Base automatically changed from next to master November 15, 2022 00:55
@alexander-akait
Copy link
Member

Let's close in favor #3436, I think we will start migration everywhere on ESM when Node.js@18 will be outdated, because there are some bugs in Node.js@18 with ESM and especial with import(...), we can make a pain for many developers if we do it right now, anyway thank you

@alexander-akait alexander-akait deleted the move-to-esm branch February 20, 2024 16:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants