-
-
Notifications
You must be signed in to change notification settings - Fork 199
Make webpack-dev-server optional #1336
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
Make webpack-dev-server optional #1336
Conversation
try { | ||
featuresHelper.ensurePackagesExistAndAreCorrectVersion('webpack-dev-server', 'the webpack Development Server'); | ||
} catch (e) { | ||
console.log(e); | ||
process.exit(1); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I could have used isPackageInstalled()
, but I would have to rewrite a lot of logic here to enforce the version, generate the good command, etc...
Adding method
parameter to featuresHelper.ensurePackagesExistAndAreCorrectVersion
was a lot easier and clean.
1196ea9
to
fd49f7d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
types relying on import('webpack-dev-server').Configuration
need to be changed to use object
instead, as our type declarations cannot depend on optional dependencies
fd49f7d
to
c7dee41
Compare
You think so? I kept |
f51585a
to
ca0bf2f
Compare
Ah, I didn't think about that, I will change that, thanks |
f4cce95
to
34588b4
Compare
34588b4
to
600c943
Compare
Checks are finally green. |
…8494) Description ----------- ### Description The release of webpack-encore v5 dropped the webpack-dev-server as a dependency and is considered a BC Break, basically why our current config doesn't work: #8371 (comment) > yes it is unrelated, but I noticed it does not work at all 🙃 * https://github.com/symfony/webpack-encore/releases/tag/v5.0.0 * See symfony/webpack-encore#1318 * See symfony/webpack-encore#1336 This PR adds `webpack-dev-server` in v5 as a dependency and adds some configuration to enable hot module reload.  ___ ### Running one devServer for both configurations The current configuration within the `webpack.config.js` checks for the CSS files within the following two paths: * core-bundle/assets/styles/* * core-bundle/contao/themes/flexible/styles/* Changes that are detected in these styles will trigger the reload of the page, thus making it easier for us to work with our assets. As of right now we have 2 Encore configurations, thus kicking out the devServer from the second configuration (as you would have to tie it to another port). The first configuration actually embeds the output path of the backend theme `flexible` so it works both ways. ___ ### Should this be a Bugfix and backported to 5.3? The dev-server has been broken since changing to webpack-encore ^v5 but this is more for DX when working on contao. Backporting this change could be done in a separate PR but I don't think it's really necessary. Commits ------- af290cf Re-Add webpack-dev-server and configure HMR eb15149 Rename the backend.js asset 921ef56 Also consider the `themes/flexible` icons and fonts for HMR 2825107 Trigger pcss compilation on imported files 8add074 Match the webpack configuration for the new IconMinimizer feature 65d6e70 Watch the `core-bundle/contao` directory for template changes as well c4933af Also add the new `contao-backend` asset to all occurrences e84e01a Update the package.lock a7b6b8f Update package-lock 5545373 Revert the `contao-backend` entry and update the public path in DevSe… e6a0964 Update `package-lock.json` 46af7a7 Revert and rebuild the assets 4c85023 Simplify webpack configuration 9818a8d Update webpack configuration d051785 Remove the trailing slashes and uncomment the live reload for HMR
… #8494) Description ----------- ### Description The release of webpack-encore v5 dropped the webpack-dev-server as a dependency and is considered a BC Break, basically why our current config doesn't work: contao/contao#8371 (comment) > yes it is unrelated, but I noticed it does not work at all 🙃 * https://github.com/symfony/webpack-encore/releases/tag/v5.0.0 * See symfony/webpack-encore#1318 * See symfony/webpack-encore#1336 This PR adds `webpack-dev-server` in v5 as a dependency and adds some configuration to enable hot module reload.  ___ ### Running one devServer for both configurations The current configuration within the `webpack.config.js` checks for the CSS files within the following two paths: * core-bundle/assets/styles/* * core-bundle/contao/themes/flexible/styles/* Changes that are detected in these styles will trigger the reload of the page, thus making it easier for us to work with our assets. As of right now we have 2 Encore configurations, thus kicking out the devServer from the second configuration (as you would have to tie it to another port). The first configuration actually embeds the output path of the backend theme `flexible` so it works both ways. ___ ### Should this be a Bugfix and backported to 5.3? The dev-server has been broken since changing to webpack-encore ^v5 but this is more for DX when working on contao. Backporting this change could be done in a separate PR but I don't think it's really necessary. Commits ------- af290cf7 Re-Add webpack-dev-server and configure HMR eb151492 Rename the backend.js asset 921ef56f Also consider the `themes/flexible` icons and fonts for HMR 2825107e Trigger pcss compilation on imported files 8add0747 Match the webpack configuration for the new IconMinimizer feature 65d6e704 Watch the `core-bundle/contao` directory for template changes as well c4933afb Also add the new `contao-backend` asset to all occurrences e84e01ab Update the package.lock a7b6b8f6 Update package-lock 5545373b Revert the `contao-backend` entry and update the public path in DevSe… e6a09647 Update `package-lock.json` 46af7a7a Revert and rebuild the assets 4c850230 Simplify webpack configuration 9818a8d1 Update webpack configuration d0517850 Remove the trailing slashes and uncomment the live reload for HMR
As discussed with @stof, we want to make the webpack-dev-server an optional peer dependency:
JavaScript dependencies are problematic, depending on many sub-dependencies, which in turn depend on more sub-dependencies, and so on... Welcome to the dependency hell!
Even if the dev-server functionality isn't used, the dependency tree is immensely more complex (over 300 additional dependencies), but this is an open door to security holes present in “discrete” (say “little-known”), but over-used dependencies.
In recent months, a sort of “witch-hunt” has been set up by some people in the JavaScript ecosystem, to replace sub-dependencies with lighter alternatives (either another dependency, or a native version) in popular project. I've started doing this on Encore for a few dependencies, and making the webpack-dev-server optional is a big win.
When upgrading Encore to v5, end-users will have to install the
webpack-dev-server
back to use it again.