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.

Sign up for GitHub

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

Jump to bottom

Build broke with 4.13.0 (sorry, this is a weird report) #271

Closed
boutell opened this issue Sep 20, 2021 · 15 comments
Closed

Build broke with 4.13.0 (sorry, this is a weird report) #271

boutell opened this issue Sep 20, 2021 · 15 comments

Comments

@boutell
Copy link

boutell commented Sep 20, 2021

I know this is unhelpful and annoying, but... my build broke with 4.13.0. And all I know for sure is that 4.12.1 fixes it.

My large and unrelated application that is not a simple test app (sorry!) silently fails to produce a webpack output js file when using 4.13.0. webpack claims success, but... the file does not exist.

How could this be your fault? I surely don't know! I looked at the differences between 4.12.1 and 4.13.0 and I was not enlightened.

So I'd be very "uh... not my bug" if I were you, but if this turns out to be the first bug report of many then perhaps something is odd after all.

The good news is, the project that falls over is at least open source, and I can answer questions about the rest of its webpack config and so on.

This procedure will reproduce the issue:

# WORKS FINE, with 4.12.1
git clone https://github.com/apostrophecms/a3-demo
cd a3-demo
npm install
npm install [email protected]
NODE_ENV=production node app @apostrophecms/asset:build

# BUT THEN BLOWS UP, with 4.13.0
npm install [email protected]
NODE_ENV=production node app @apostrophecms/asset:build

(You'll get an error that apos-build.js does not exist, even though it's the configured output file of the build)

I found the module responsible by trial and error.

(I structured the reproduction instructions above so that they should still succeed and fail in that order even after we temporarily pin our dependency to 4.12.1.)

We're currently pinned on webpack 5.44.x, but I tried latest with the same result.

Thanks for your time.
@robcresswell
Copy link
Owner

robcresswell commented Sep 23, 2021 •
edited
Loading

@boutell First off, thanks so much for the report! I'm really sorry if I've broken something. I'm struggling a little bit to see where I could've broken something; here's some of the icons, for example:

In 4.12.1 https://unpkg.com/browse/[email protected]/Android.vue

In 4.13.0 https://unpkg.com/browse/[email protected]/Android.vue

I'll pull down your code and check it, I think I'll have time at the weekend to do this. Is that okay with you? Thanks again for the repro

@boutell
Copy link
Author

boutell commented Sep 23, 2021 via email

@raimund-schluessler
Copy link

Not sure that this is the problem (I didnt' check your app), but some icons got renamed in the material design icons package. E.g. HandLeft is now named HandBackLeft and importing HandLeft will fail and break the compilation (technically, for that reason 4.13.0 should have been 5.0.0 if following semver 😉). Although webpack should properly warn about the missing import, it might still be worth to check that the icons are still available under the previous name.

@robcresswell
Copy link
Owner

(technically, for that reason 4.13.0 should have been 5.0.0 if following semver 😉).

Ah, this is a good point. I've never tested for the generated icons of a newer version being a superset of the previous versions.

@boutell
Copy link
Author

boutell commented Sep 28, 2021

Oh, this is very likely to be it, actually. We definitely are relying on these not to change name or disappear in the midst of a major version.

Of course the next issue is then why our build system would fail silently and claim success (!!!), but that part is not your problem (:

@boutell
Copy link
Author

boutell commented Sep 28, 2021

For now we are pinned to a previous release.

@a-kriya
Copy link

a-kriya commented Oct 11, 2021

Matches for "pdf" in 4.12.1:

FilePdf.vue
FilePdfBox.vue
FilePdfBoxOutline.vue
FilePdfOutline.vue
PdfBox.vue

In 4.13.0, just FilePdfBox.vue remains.

@robcresswell
Copy link
Owner

@a-kriya It seems like a lot of the upstream icons must've been renamed. All my repo does is wrap them into components for ease of use.

I think the easiest thing to do going forward is to match the upstream versioning for breaking changes. As in, if upstream publishes a breaking change, I'll do the same, regardless of reason. I don't know how I'd determine the reason anyway; there are no changelogs or usable commit history.

@a-kriya
Copy link

a-kriya commented Oct 12, 2021 •
edited
Loading

@robcresswell Templarian/MaterialDesign#5409 lists all the icons that were removed and renamed. These PDF ones had Adobe Acrobat logo so they were intentionally removed.

In the above linked post, they mention that they're not following semver, so you will likely have to manually bump up major for anything breaking.

@robcresswell
Copy link
Owner

@a-kriya Ah, nice find... I hate when people use GitHub for listing changes but not git itself 🙈

Going forward I'll publish breaking changes when icons change.

@boutell
Copy link
Author

boutell commented Oct 13, 2021

Thank you for your hard work on this!

@robcresswell
Copy link
Owner

Closing because I've said I'll change the behaviour going forward, but I don't think there's anything to do otherwise

@boutell
Copy link
Author

boutell commented Dec 1, 2021

That's fair! We'll look at getting back into sync with your icon names so we can follow you via semver.

@robcresswell
Copy link
Owner

Thanks @boutell. Sorry again for the trouble!

@boutell
Copy link
Author

boutell commented Dec 1, 2021 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@boutell @raimund-schluessler @robcresswell @a-kriya