-
-
Notifications
You must be signed in to change notification settings - Fork 198
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
Add new Travis jobs to test lowest/highest versions of all the dependencies #403
Add new Travis jobs to test lowest/highest versions of all the dependencies #403
Conversation
be181c3
to
7abea75
Compare
// <package>@<version> '<version>' | ||
if (parts.length === 2) { | ||
resolve([dependency, parts[1].replace(/'/g, '')]); | ||
} |
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.
Wow, yea, this is nuts! But, if it works, and it's just a build process, let's use it until there is a better way
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.
Yeah, I wasn't really happy with that part... I guess calling the npm.js API directly would have been a cleaner way to do it but then we would also have had to filter the versions ourselves (and since it happens before npm install
/yarn
, we don't have access to the semver
package at that point).
Thank you for highlighting that part by the way, I noticed that I forgot some return
in promises... not a big issue since a promise won't be able to change its state after being resolved/rejected but still something that had to be fixed :)
@@ -0,0 +1,128 @@ | |||
/* |
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.
Would it be worth to put this into a separate npm package? This sounds like something that more developers would run into when maintaining packages.
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.
as this is meant to run before a npm install
, it might be harder 😄
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.
Maybe something like a global install? Or a phar like equivalent for js
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'm always interested in re-using existing libs or extracting Encore into new libs. But, with limited resources, we need to stay focused on Encore itself (i.e. things like this would need to be done by some community member).
Thanks @Lyrkan! This is a big step forward imo! |
… the dependencies (Lyrkan) This PR was squashed before being merged into the master branch (closes #403). Discussion ---------- Add new Travis jobs to test lowest/highest versions of all the dependencies This PR adds two Travis jobs that allow to test the lowest and highest versions of all the dependencies (including the dev ones since the version ranges are used by the package helper). I wasn't sure about the method to use for the lowest versions since we can't really guess them without calling the npm's registry API. So, if someone has a better idea... :) Note that both jobs currently fail for the following reasons: - Lowest versions: This isn't really an issue since it seems related to Webpack 4 for which we haven't released a version yet. We could increase the minimum version to match the one from the `yarn.lock` file before releasing. - Highest versions: This is caused by the last version of the `mini-css-extract-plugin` (0.4.3) which doesn't seem to work well with the `webpack-manifest-plugin` anymore (see webpack-contrib/mini-css-extract-plugin#177 (comment)). Closes #39. Commits ------- bcabda6 Add some missing 'return' statements 0161000 Remove hardcoded sass-loader version in addPackagesVersionConstraint test 7abea75 Add new Travis jobs to test lowest/highest versions of all the dependencies
…. allowed Webpack version (Lyrkan) This PR was merged into the master branch. Discussion ---------- Fix the mini-css-extract-plugin 0.4.3 issue and increase min. allowed Webpack version This PR fixes both of the issues detected in #403: - Set the maximum allowed version for `mini-css-extract-plugin` to 0.4.2 (since 0.4.3 doesn't work with the current version of the webpack-manifest-plugin, see #404). This is a temporary solution, once it's fixed (either in mini-css-extract-plugin or in weback-manifest-plugin) we'll have to change it again. - Set the minimum allowed version for `webpack` to 4.20.0. The previous one (4.0.0) seemed to break `mini-css-extract-plugin` (see https://travis-ci.org/Lyrkan/webpack-encore/jobs/439810002). Commits ------- 04f42af Temporary fix for the mini-css-extract-plugin 0.4.3 issue
This PR adds two Travis jobs that allow to test the lowest and highest versions of all the dependencies (including the dev ones since the version ranges are used by the package helper).
I wasn't sure about the method to use for the lowest versions since we can't really guess them without calling the npm's registry API. So, if someone has a better idea... :)
Note that both jobs currently fail for the following reasons:
yarn.lock
file before releasing.mini-css-extract-plugin
(0.4.3) which doesn't seem to work well with thewebpack-manifest-plugin
anymore (see fix(loader): passemitFile
to the child compilation (loaderContext.emitFile
) webpack-contrib/mini-css-extract-plugin#177 (comment)).Closes #39.