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

support webpack.config.cjs by default #1165

Closed
xiaoxiangmoe opened this issue Jan 9, 2020 · 22 comments · Fixed by #1775
Closed

support webpack.config.cjs by default #1165

xiaoxiangmoe opened this issue Jan 9, 2020 · 22 comments · Fixed by #1775
Labels

Comments

@xiaoxiangmoe
Copy link

If a webpack.config.js is present, the webpack command picks it up by default. We use the --config option here only to show that you can pass a config of any name. This will be useful for more complex configurations that need to be split into multiple files.

We should also pick it up when a webpack.config.cjs is present

@ematipico
Copy link
Contributor

ematipico commented Jan 17, 2020

This requires a minimum version of node right?

@xiaoxiangmoe
Copy link
Author

xiaoxiangmoe commented Jan 17, 2020

No, we can just regard cjs as js

@smelukov
Copy link
Member

Seems like we can't just support it - gulpjs/interpret#65

@ExE-Boss
Copy link

ExE-Boss commented Feb 19, 2020

Actually, you can, as .cjs can be used anywhere a .js file can be used currently, which both the docs and the source code confirm.

@Parikshit-Hooda
Copy link
Contributor

Hi. Can I take up this issue? @ematipico

@Parikshit-Hooda
Copy link
Contributor

Hi. I have made some changes, how do I test that the changes resolve the issue?
Specifically, I have made changes to the 'packages/webpack-cli/lib/groups/configGroup.js' file to include support for '.cjs' file. Thanks.

@snitin315
Copy link
Member

how do I test that the changes resolve the issue

@Parikshit-Hooda As this is a new feature you are implementing you have to write your own tests.

Also, you can run the current tests with yarn test

@Parikshit-Hooda
Copy link
Contributor

@snitin315 Actually, I want to confirm if the feature addition actually fixes this issue or not? I imagine that the modified repo(fixed webpack-cli) will need to be imported in a dummy project and check. How do I go about checking this?

@snitin315
Copy link
Member

you can use npm link or yarn link

@Parikshit-Hooda
Copy link
Contributor

Haven't done this kind of an activity before. Can you please elaborate or tell me what exactly should I google?

@anshumanv
Copy link
Member

Haven't done this kind of an activity before. Can you please elaborate or tell me what exactly should I google?

You need to run your changes locally, https://docs.npmjs.com/cli/link.html
Link the package and try to run with a .cjs config file then write tests for it.

@Parikshit-Hooda
Copy link
Contributor

Question: Should I link the repo after following the contributing.md and making edits or without following the instructions in CONTRIBUTING.md. I am asking because I am not sure if going through yarn install ->bootstrap -> build causes some unrequired changes.

@anshumanv
Copy link
Member

anshumanv commented Mar 16, 2020

build causes some unrequired changes.

What are the unrequired changes?

Yep please install, bootstrap, build then link the webpack-cli package in the packages folder.

Then you need to npm link webpack-cli in a folder, that folder will then use the local code that you have.

Question: Should I link the repo after following the contributing.md and making edits or without following the instructions in CONTRIBUTING.md.

Yes, after following the steps.

@Parikshit-Hooda
Copy link
Contributor

Capture
After linking it, its still requiring me to 'npm install webpack-cli' to run the npm run build. What am I doing wrong?

@anshumanv
Copy link
Member

Invoke using webpack-cli, after you run npm link webpack-cli in your test folder.

@Parikshit-Hooda
Copy link
Contributor

Doesn't seem to work.
$ webpack-cli --entry src/index.js --config webpack.config.cjs
bash: webpack-cli: command not found

@anshumanv
Copy link
Member

You didn't symlink properly then.

@Parikshit-Hooda
Copy link
Contributor

got it. will raise a PR in a couple of days.

@Parikshit-Hooda
Copy link
Contributor

Parikshit-Hooda commented Mar 17, 2020

Alternative solution: wondering if replacing the extension '.cjs' with '.js' is a better solution?
Reason: I was going through the file that is to be edited and 'webpack.config.js' is actually used as a primitive default variable and the variable scope is dispersed and large. changing the extension is hence a simpler solution. Also, if anybody has a suggestion which files actually need to be inspected for this feature, do let me know!

@ExE-Boss
Copy link

ExE-Boss commented Mar 18, 2020

.js is parsed as an ECMAScript module when package.json has type: "module" specified.

.cjs is always parsed as a CommonJS module, which is the only module format which webpack currently supports for its configuration files.

@anshumanv
Copy link
Member

^
+

Alternative solution: wondering if replacing the extension '.cjs' with '.js' is a better solution?

Nope, as stated above they are interpreted differently, so not a good solution.

@tcodes0
Copy link

tcodes0 commented Sep 9, 2020

.js is parsed as an ECMAScript module when package.json has type: "module" specified.

.cjs is always parsed as a CommonJS module, which is the only module format which webpack currently supports for its configuration files.

Hey yall. Is it currently planned to support esm for config files? I guess it would raise the minimum node version way too high right?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants