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

Add new Travis jobs to test lowest/highest versions of all the dependencies #403

Merged
merged 3 commits into from Oct 18, 2018

Conversation

Lyrkan
Copy link
Collaborator

@Lyrkan Lyrkan commented Oct 7, 2018

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:

Closes #39.

// <package>@<version> '<version>'
if (parts.length === 2) {
resolve([dependency, parts[1].replace(/'/g, '')]);
}
Copy link
Member

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

Copy link
Collaborator Author

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 @@
/*
Copy link
Contributor

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.

Copy link
Member

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 😄

Copy link
Contributor

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

Copy link
Member

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).

@weaverryan
Copy link
Member

Thanks @Lyrkan! This is a big step forward imo!

@weaverryan weaverryan merged commit bcabda6 into symfony:master Oct 18, 2018
weaverryan added a commit that referenced this pull request Oct 18, 2018
… 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
weaverryan added a commit that referenced this pull request Oct 18, 2018
…. 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants