Skip to content

🐛 Add all chunks from stats object#20

Merged
themgoncalves merged 1 commit intothemgoncalves:masterfrom
lex111:all-chunks
May 29, 2020
Merged

🐛 Add all chunks from stats object#20
themgoncalves merged 1 commit intothemgoncalves:masterfrom
lex111:all-chunks

Conversation

@lex111
Copy link
Copy Markdown
Contributor

@lex111 lex111 commented May 18, 2020

Summary

In my project, I encountered an error (see below), in the end I found out that not all chunks are added to manifest, I tried to fix it.

Why

In my code, I ran into a problem when the chunk is used, but was not defined in the assets property, which is why I got the following error (from getBundles).

TypeError: Cannot convert undefined or null to object
    at Function.keys (<anonymous>)
    at Array.reduce (<anonymous>)TypeError: Cannot convert undefined or null to object
    at Function.keys (<anonymous>)
    at Array.reduce (<anonymous>)

As a rule, chunk.ids contains only one element - this is chunk.id, but there are cases when there may be several ids, and it seems to me that we should take this fact into account.

@themgoncalves honestly, I don’t know for sure if this solution is right, but it solved my problem. I would be grateful if you could help me improve this change.

Checklist

  • Your code builds clean without any errors or warnings
  • You are using approved terminology
  • You have added unit tests, if apply.

@lex111
Copy link
Copy Markdown
Contributor Author

lex111 commented May 28, 2020

Hi @themgoncalves, any thoughts about this PR?

@themgoncalves themgoncalves self-requested a review May 28, 2020 15:00
@themgoncalves themgoncalves added bug Something isn't working investigation This issue needs an investigation labels May 28, 2020
@themgoncalves
Copy link
Copy Markdown
Owner

themgoncalves commented May 28, 2020

Hi @lex111,

Thanks for your contribution, I will be debugging the code and making sure that we got it all covered, but for that, I need to have more infos so I can a test environment for that.

Do you mind to share a code demo that runs the issue you reported?

@lex111
Copy link
Copy Markdown
Contributor Author

lex111 commented May 28, 2020

We use your lib in Docusaurus, you probably remember this project.

Reproducible demo you can find here https://github.com/facebook/docusaurus/tree/repro-2244/website-build-fail

When you try to build that site by running the yarn build command, you get the error that I indicated in the issue.

This happens when the user overrides the NotFound component (see https://v2.docusaurus.io/docs/using-themes/#swizzling-theme-components). If you delete this file, then there will be no fatal errors when building the site.

I don’t understand why this is happening, I just found out that it is due to react-loadable-ssr-addon
So I will be very glad if you can help with this strange error. I hope you have enough information for debugging?

@themgoncalves
Copy link
Copy Markdown
Owner

themgoncalves commented May 28, 2020

Yes, this is enough information. I’ll be getting back to this topic tomorrow.

Thanks for sharing such detailed infos.

themgoncalves
themgoncalves approved these changes May 29, 2020
View reviewed changes
Copy link
Copy Markdown
Owner

@themgoncalves themgoncalves left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code wise and functionality looks good.
Couldn't find any possible issues that this PR could introduce, so we're good to go.

@themgoncalves themgoncalves merged commit 7f64458 into themgoncalves:master May 29, 2020
@lex111
Copy link
Copy Markdown
Contributor Author

lex111 commented May 29, 2020

@themgoncalves thank you! Although I do not understand why and how more elements can appear in chunks array. But in any case, this PR should not bring breaking changes. Could you please release a new version?

@themgoncalves
Copy link
Copy Markdown
Owner

themgoncalves commented May 29, 2020

@lex111 v0.2.1 was released: https://www.npmjs.com/package/react-loadable-ssr-addon

@themgoncalves
Copy link
Copy Markdown
Owner

themgoncalves commented May 29, 2020

@lex111 As precaution measure I just release another patch with a few packages downgrade, as it was showing flaky behaviour which concerns me.

@lex111
Copy link
Copy Markdown
Contributor Author

lex111 commented May 29, 2020

@themgoncalves I got you, so I will use v0.2.2 then.

@lex111 lex111 mentioned this pull request Jun 1, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@themgoncalves themgoncalves themgoncalves approved these changes

Assignees

No one assigned

Labels

bug Something isn't working investigation This issue needs an investigation

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@lex111 @themgoncalves