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

fix(webpack-cli): add an option for preventing interpret #3329

Merged
merged 14 commits into from
Jul 25, 2022

Conversation

shuta13
Copy link
Contributor

@shuta13 shuta13 commented Jul 6, 2022

What kind of change does this PR introduce?

A kind of bugfix.

Did you add tests for your changes?

Yes.

If relevant, did you update the documentation?

Users can check this option with the webpack --help.

Summary

Fixed #3195

Does this PR introduce a breaking change?

No.

Other information

@shuta13 shuta13 requested a review from a team as a code owner July 6, 2022 06:12
@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Jul 6, 2022

CLA Signed

The committers listed above are authorized under a signed CLA.

  • ✅ login: shuta13 / name: Shuta Hirai (6cbe9d3)

@shuta13 shuta13 requested a review from snitin315 July 8, 2022 08:20
@shuta13
Copy link
Contributor Author

shuta13 commented Jul 11, 2022

@snitin315 ↑ Please check those.

OPTIONS.md Outdated Show resolved Hide resolved
Copy link
Member

@rishabh3112 rishabh3112 left a comment

Choose a reason for hiding this comment

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

Left a suggestion to complete the testing cases.

test/build/config-format/typescript/typescript.test.js Outdated Show resolved Hide resolved
anshumanv
anshumanv previously approved these changes Jul 18, 2022
Copy link
Member

@anshumanv anshumanv left a comment

Choose a reason for hiding this comment

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

いい感じ

OPTIONS.md Outdated Show resolved Hide resolved
Co-authored-by: Anshuman Verma <anshu.av97@gmail.com>
@shuta13
Copy link
Contributor Author

shuta13 commented Jul 22, 2022

@snitin315

Sorry for the late reply. I added the way to remove --require from yarn test:coverage without testing error : 645bb7b

Please check and rerun the CI workflows.

@anshumanv
Copy link
Member

スタートしました〜

@codecov
Copy link

codecov bot commented Jul 22, 2022

Codecov Report

Merging #3329 (c1763f0) into master (58503be) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #3329   +/-   ##
=======================================
  Coverage   92.26%   92.27%           
=======================================
  Files          23       23           
  Lines        1733     1734    +1     
  Branches      519      520    +1     
=======================================
+ Hits         1599     1600    +1     
  Misses        134      134           
Impacted Files Coverage Δ
packages/webpack-cli/src/webpack-cli.ts 93.96% <100.00%> (+<0.01%) ⬆️

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 58503be...c1763f0. Read the comment docs.

@shuta13
Copy link
Contributor Author

shuta13 commented Jul 22, 2022

I forgot to update snapshots when testing with dev-server@3. : https://github.com/webpack/webpack-cli/runs/7462801069?check_suite_focus=true#step:9:96

I'll fix...

@anshumanv
Copy link
Member

It seems other tests are passing though which is good

@shuta13 shuta13 requested a review from snitin315 July 22, 2022 06:08
@shuta13
Copy link
Contributor Author

shuta13 commented Jul 22, 2022

@anshumanv

It seems other tests are passing though which is good

Thx. But, I'm worried about other contributors may be confused with the CI failing even though they don't make any changes. So, I update snapshots 🙏 : c1763f0

@anshumanv
Copy link
Member

Thanks I've started CI, let's see if everything passes this time 🤞

Copy link
Member

@snitin315 snitin315 left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for contributing ⭐.

/cc @alexander-akait

Copy link
Member

@alexander-akait alexander-akait left a comment

Choose a reason for hiding this comment

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

@snitin315 After merge we will need to rebase the next branch, this can be a problem, maybe we will merge this directly in next?

@snitin315
Copy link
Member

@alexander-akait Yes, let's point this PR to the next branch.

@snitin315 snitin315 changed the base branch from master to next July 25, 2022 10:44
Copy link
Member

@alexander-akait alexander-akait left a comment

Choose a reason for hiding this comment

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

Feel free to merge

@snitin315 snitin315 merged commit 222c96d into webpack:next Jul 25, 2022
snitin315 added a commit that referenced this pull request Jul 25, 2022
* 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 <anshu.av97@gmail.com>

* refactor: built-in options type

Co-authored-by: Nitin Kumar <snitin315@gmail.com>

* test: re-update snapshots

* fix: add double quote

Co-authored-by: Nitin Kumar <snitin315@gmail.com>

* test: update snapshots for webpack4

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

* test: update snapshots

Co-authored-by: Anshuman Verma <anshu.av97@gmail.com>
Co-authored-by: Nitin Kumar <snitin315@gmail.com>
snitin315 added a commit that referenced this pull request Sep 10, 2022
* 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 <anshu.av97@gmail.com>

* refactor: built-in options type

Co-authored-by: Nitin Kumar <snitin315@gmail.com>

* test: re-update snapshots

* fix: add double quote

Co-authored-by: Nitin Kumar <snitin315@gmail.com>

* test: update snapshots for webpack4

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

* test: update snapshots

Co-authored-by: Anshuman Verma <anshu.av97@gmail.com>
Co-authored-by: Nitin Kumar <snitin315@gmail.com>
snitin315 added a commit that referenced this pull request Sep 17, 2022
* 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 <anshu.av97@gmail.com>

* refactor: built-in options type

Co-authored-by: Nitin Kumar <snitin315@gmail.com>

* test: re-update snapshots

* fix: add double quote

Co-authored-by: Nitin Kumar <snitin315@gmail.com>

* test: update snapshots for webpack4

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

* test: update snapshots

Co-authored-by: Anshuman Verma <anshu.av97@gmail.com>
Co-authored-by: Nitin Kumar <snitin315@gmail.com>
snitin315 added a commit that referenced this pull request Sep 24, 2022
* 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 <anshu.av97@gmail.com>

* refactor: built-in options type

Co-authored-by: Nitin Kumar <snitin315@gmail.com>

* test: re-update snapshots

* fix: add double quote

Co-authored-by: Nitin Kumar <snitin315@gmail.com>

* test: update snapshots for webpack4

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

* test: update snapshots

Co-authored-by: Anshuman Verma <anshu.av97@gmail.com>
Co-authored-by: Nitin Kumar <snitin315@gmail.com>
snitin315 added a commit that referenced this pull request Sep 24, 2022
* 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 <anshu.av97@gmail.com>

* refactor: built-in options type

Co-authored-by: Nitin Kumar <snitin315@gmail.com>

* test: re-update snapshots

* fix: add double quote

Co-authored-by: Nitin Kumar <snitin315@gmail.com>

* test: update snapshots for webpack4

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

* test: update snapshots

Co-authored-by: Anshuman Verma <anshu.av97@gmail.com>
Co-authored-by: Nitin Kumar <snitin315@gmail.com>
snitin315 added a commit that referenced this pull request Oct 8, 2022
* 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 <anshu.av97@gmail.com>

* refactor: built-in options type

Co-authored-by: Nitin Kumar <snitin315@gmail.com>

* test: re-update snapshots

* fix: add double quote

Co-authored-by: Nitin Kumar <snitin315@gmail.com>

* test: update snapshots for webpack4

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

* test: update snapshots

Co-authored-by: Anshuman Verma <anshu.av97@gmail.com>
Co-authored-by: Nitin Kumar <snitin315@gmail.com>
snitin315 added a commit that referenced this pull request Oct 22, 2022
* 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 <anshu.av97@gmail.com>

* refactor: built-in options type

Co-authored-by: Nitin Kumar <snitin315@gmail.com>

* test: re-update snapshots

* fix: add double quote

Co-authored-by: Nitin Kumar <snitin315@gmail.com>

* test: update snapshots for webpack4

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

* test: update snapshots

Co-authored-by: Anshuman Verma <anshu.av97@gmail.com>
Co-authored-by: Nitin Kumar <snitin315@gmail.com>
snitin315 added a commit that referenced this pull request Nov 6, 2022
* 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 <anshu.av97@gmail.com>

* refactor: built-in options type

Co-authored-by: Nitin Kumar <snitin315@gmail.com>

* test: re-update snapshots

* fix: add double quote

Co-authored-by: Nitin Kumar <snitin315@gmail.com>

* test: update snapshots for webpack4

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

* test: update snapshots

Co-authored-by: Anshuman Verma <anshu.av97@gmail.com>
Co-authored-by: Nitin Kumar <snitin315@gmail.com>
snitin315 added a commit that referenced this pull request Nov 8, 2022
* 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 <anshu.av97@gmail.com>

* refactor: built-in options type

Co-authored-by: Nitin Kumar <snitin315@gmail.com>

* test: re-update snapshots

* fix: add double quote

Co-authored-by: Nitin Kumar <snitin315@gmail.com>

* test: update snapshots for webpack4

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

* test: update snapshots

Co-authored-by: Anshuman Verma <anshu.av97@gmail.com>
Co-authored-by: Nitin Kumar <snitin315@gmail.com>
alexander-akait pushed a commit that referenced this pull request Nov 15, 2022
* 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 <anshu.av97@gmail.com>

* refactor: built-in options type

Co-authored-by: Nitin Kumar <snitin315@gmail.com>

* test: re-update snapshots

* fix: add double quote

Co-authored-by: Nitin Kumar <snitin315@gmail.com>

* test: update snapshots for webpack4

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

* test: update snapshots

Co-authored-by: Anshuman Verma <anshu.av97@gmail.com>
Co-authored-by: Nitin Kumar <snitin315@gmail.com>
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.

no way to prevent interpret from transpiling the config, even if another tool has been configured
6 participants