Skip to content
This repository has been archived by the owner on Aug 3, 2023. It is now read-only.

feat #323: Allow fn & promise from webpack config #325

Conversation

third774
Copy link
Contributor

PR for #323

@ashleygwilliams
Copy link
Contributor

@xtuc can you please review this PR?

tests/build.rs Outdated Show resolved Hide resolved
@xtuc
Copy link
Member

xtuc commented Jul 29, 2019

Could you please add the env and argv params from the doc https://webpack.js.org/configuration/configuration-types/#exporting-a-function?

Also, It seems to me that we are duplicating the webpack-cli logic. Do you know if there a way to share some logic/code? We are fine having it as dependecy.

@third774
Copy link
Contributor Author

Thanks for the feedback! I'll see what I can find on the webpack front.

@third774
Copy link
Contributor Author

Would this mean we would allow for CLI arguments used on the wrangler cli to be passed to webpack? Or maybe filter down to a list of webpack specific flags?

For example, if I run wrangler build --env.mode="dev", should I expect to see that in my env object?

@xtuc
Copy link
Member

xtuc commented Aug 9, 2019

That's a good question. I don't think we should support all the webpack-cli arguments in wrangler. I would pass an empty object. Would it make sense to you?

Also note that in #220 we are going to support passing an environment, it would be great if webpack could get access to it.

@third774
Copy link
Contributor Author

I think passing an empty object for env makes sense for now. Once #220 is complete, it could definitely make sense to make that available as well.

Additionally, there could be some defaults that get set and merged with webpack-merge. I believe webpack-cli does this under the hood for providing the mode along with a few other things.

@third774 third774 force-pushed the issue-323-function-and-promise-support-for-webpack branch from 28bb6c3 to bdc59b5 Compare August 11, 2019 16:34
wranglerjs/index.js Outdated Show resolved Hide resolved
@xtuc
Copy link
Member

xtuc commented Aug 15, 2019

The PR looks good to me, I would like to wait for #220, so we can pass it into the env object and make it feature complete.

@third774
Copy link
Contributor Author

@xtuc — awesome! Is there anything I need to do to fix the CI tests?

@xtuc
Copy link
Member

xtuc commented Aug 16, 2019

@third774 I fixed them. I think we should wait for #385 now.

@xtuc xtuc force-pushed the issue-323-function-and-promise-support-for-webpack branch from a133aeb to 53bb60e Compare September 2, 2019 16:22
@xtuc xtuc removed the blocked label Sep 2, 2019
@xtuc xtuc merged commit a5b1dbe into cloudflare:master Sep 2, 2019
@xtuc
Copy link
Member

xtuc commented Sep 2, 2019

@third774 let's maybe not wait for env to land. I went ahead a merged this change, it will be release soon.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
webpack Issues that involve the `webpack` bundler
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants