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

Replace the webpack-stream dependency #11996

Closed
Snuffleupagus opened this issue Jun 12, 2020 · 4 comments · Fixed by #13008
Closed

Replace the webpack-stream dependency #11996

Snuffleupagus opened this issue Jun 12, 2020 · 4 comments · Fixed by #13008

Comments

@Snuffleupagus
Copy link
Collaborator

With PR #11436 we became locked at a particular (older) version of the webpack-stream dependency, because of bugs in the latest versions; see #11436 (comment) for details.

Looking at the GitHub-page for webpack-stream, it unfortunately seems that the project may be unmaintained/dead at this point given that the latest commit was in 2018.
Hence, rather than trying to update webpack-stream it's probably better to attempt to replace it completely instead (possibly with our own code if necessary) to ensure that we'll be able to e.g. update to Webpack 5 when it's released (which will likely be needed to properly support new ECMAScript features).

@Snuffleupagus
Copy link
Collaborator Author

This is now becoming more of a problem, and it should ideally be prioritized, since I recently tried to add support for optional chaining but unfortunately ran into errors in webpack-stream.

@timvandermeij
Copy link
Contributor

This dependency is now updated to the most recent version in #12397. However, given that's it's a very inactively maintained dependency I also think we should get rid of it. Moreover, we only seem to be using it in one place in the Gulpfile...

@timvandermeij
Copy link
Contributor

timvandermeij commented Feb 13, 2021

Now that we're using Webpack 5, the dependency got updated and there was more development on their GitHub page, do we still want to do this?

I guess we could make https://github.com/mozilla/pdf.js/blob/master/package.json#L61 use ^ instead of ~ now.

@Snuffleupagus
Copy link
Collaborator Author

Snuffleupagus commented Feb 13, 2021

Now that we're using Webpack 5

Given that the Webpack upgrade itself wasn't actually blocked by the webpack-stream dependency, as I incorrectly assumed previously, I suppose this reduces the severity of this issue considerably.

the dependency got updated and there was more development on their GitHub page, do we still want to do this?

To me, this all depends on whether or not they'll try to re-introduce the broken caching mechanism (which originally forced us to pin the version number).

I guess we could make https://github.com/mozilla/pdf.js/blob/master/package.json#L61 use ^ instead of ~ now.

I suppose that could work, as long as we remember to check webpack-stream commits for any possible breakage when updating the npm packages (which I've been doing the last few times).
If you want to submit a PR with this change, and then simply close this issue, then please be my guest :-)

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

Successfully merging a pull request may close this issue.

2 participants