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

Error: Unexpected token '<' after publishing new version #13

Closed
TarikHuber opened this issue Nov 5, 2020 · 10 comments
Closed

Error: Unexpected token '<' after publishing new version #13

TarikHuber opened this issue Nov 5, 2020 · 10 comments

Comments

@TarikHuber
Copy link

I have updated our custom CRA template to use the PWA features from this project. Previously it we used the setup from tha core CRA template. But since switching to this solution we have randomly the error "Unexpected token '<'" after publishing new versions of the application to the demo page (https://www.react-most-wanted.com/)

To recreate this bug just create a project with this template with: npx create-react-app my-app --template rmw
And publish it to a Firebase project using serve localy. I could not reproduce this issue localy. IT apears in most cases when publishing to Firebase.

Here are the screenshots of the message and files in the cache:

photo_2020-11-05_13-51-46

photo_2020-11-05_13-51-26

I have searched for solutions and tried some of them but nothing could solve the issue. Tried different entries in the package.json file for homepage. Changed the manifest.json and index.html to be exactly the same like in this template.

Basicaly the only difference between our and this template is the stuff inside the App React Component. So it's strange to me that Compoenent could have any effect on the service worker.

Could it be that the lazy loaded components in App are making this problem?
Would removing clientsClaim in the SW solve this issue?

I would rather leave the files from this template the same and unchanged if it's possible.

@jeffposnick
Copy link
Collaborator

It was a little difficult to get my webpack dependency working when using your template.

Once I did get things working, I saw the following warning printed out in the console following the build:

/static/js/2.47ac2580.chunk.js is 2.3 MB, and won't be precached. Configure maximumFileSizeToCacheInBytes to change this limit.

If you have your HTML precached and some of its lazy-loaded dependencies aren't precached, then there can be a cache miss following a redeployment. You can learn more about this issue at https://pawll.glitch.me/

Unfortunately, c-r-a still locks down your underlying webpack config, so that maximumFileSizeToCacheInBytes option in Workbox's InjectManifest plugin can't be tweaked easily.

But it does explain why changes to your React Component (which leads to the bloated chunk sizes) would lead to the behavior you're seeing.

@TarikHuber
Copy link
Author

@jeffposnick thank you so much for the fast response and the feedback 😄

Sorry for the broken template. There was an old package.json file in the template with the package nwb inside and that one messed up the webpack in the template. It should work now and have smaller chunks.

Even no chunk should be larger than 2 MB and I changed the template so it doesn't, where could I set the maximumFileSizeToCacheInBytes property to eneable larger files. I have some large projects that are used only on desktop where such chunk sizes and page loading speed are not so importand.

@jeffposnick
Copy link
Collaborator

Although c-r-a v4 allows for more control over the actual logic inside the service worker via the switch to the workbox-webpack-plugin's InjectManifest mode, the underlying InjectManifest configuration (which is where maximumFileSizeToCacheInBytes would need to be changed) still can't be customized without ejecting.

The actual code is at https://github.com/facebook/create-react-app/blob/aec42e2cc05fe0799a3b73830b874757e9e3f561/packages/react-scripts/config/webpack.config.js#L713-L717

@TarikHuber
Copy link
Author

OK. I was thinking that I'm to stupid to find where to set it. Was searching for hours 🤣
Thank you again. You saved me with this issue 😄

@jeffposnick
Copy link
Collaborator

I don't know how common it is to ship more than 2mb of code in a given chunk, but if you think that the limit is too low, feel free to file a PR against that config file in c-r-a to raise it.

There has to be some default (we don't want folks to accidentally precache a 1gb movie file...), but I have no objection to folks who know the c-r-a ecosystem more changing the default.

@TarikHuber
Copy link
Author

I agree that no chung should be larger but there should be more information about this. If someone is using this template then to get a working PWA. This limit is creating a bug that is very easy to overlook.

I would recommend 3 points:

The warning

The warning: /static/js/2.47ac2580.chunk.js is 2.3 MB, and won't be precached. Configure maximumFileSizeToCacheInBytes to change this limit.

Is shown before the list of chunks so if you don't watch the console or scroll over the list of chunks you won't see it. If you start the build, do something else , go back and done't see any warnings but the normal message that your project is build you won't scroll above to check stuff. That is the reason I didn't notice the warning. This is how my terminal looks with the warning somewhere above:

photo_2020-11-06_22-31-26

Showing the warning that one chunk will not be precached and could/will break the app (it's very hard to remove this bug on android devices) on the next update could be shown at the end or somewhere near the end of the build log. It would be good that the warning has a link to possible issues related to missing chunks in the precache.

Question?

It would help if a question appears in the build like "A chunkg is larger than X. Do you want to continue? For more info check LINK". That way the developer will definitely notice the warning.

The limit

For me it would make much more sense to use the warning to inform the developer that he has a to large chunk but still precache it. Even if it's larger then it should be. In the end it's better to precache a to large file than breaking the whole app with a white screen. If such a bug gets into production a normal user would get stuck with it for a while.

@jeffposnick
Copy link
Collaborator

The workbox-webpack-plugin reports the warning back to the main webpack compilation, but it's up to the c-r-a project how to deal with those warnings. (It logs the message to the console, as noted.)

I don't think that warning and still precaching files of every size is the best option—there really should be some upper bound. It could be more than 2mb, if it's common for c-r-a to output chunks larger than that, though.

https://pawll.glitch.me/ has some background on the things that could wrong if there's a mismatch due to a cached HTML document that lazily-loads a JavaScript resource that is no longer available. (A good course of action is to code defensively whenever you're lazily-loading anything, as other scenarios, like just having a long-lived tab, can lead to the same failure.) In this particular scenario, the issue should be resolved when the "waiting" service worker gets a chance to activate, i.e. after all tabs are closed or after skipWaiting() is called.

@TarikHuber
Copy link
Author

Unfortunately the Issue with Unexpected token '<' is still present. It doesn't happen on the initial load now but after the update and when you go deeper into the app.

Here is a screenshot:
photo_2020-11-11_09-56-47
and
photo_2020-11-11_09-57-42

There are no oversized chunks anymore. To reproduce you an use again the template npx create-react-app my-app --template rmw (it shoul work now) and re-serve it localy after some changes in the app.

The same App worked with the old c-r-a template without this issues.

It looks like the new service worker is overriding the cache of the old one without having the controll over all tabs.
Could we version the cache and delete the old one when the new one get's the status active?

@TarikHuber TarikHuber reopened this Nov 11, 2020
@jeffposnick
Copy link
Collaborator

Your template calls skipWaiting() unconditionally whenever there's an updated service worker. This isn't recommended unless you also do a page reload once the waiting service worker activates. (C.f. https://developers.google.com/web/tools/workbox/guides/advanced-recipes#offer_a_page_reload_for_users)

Here's the code in question: https://github.com/TarikHuber/react-most-wanted/blob/master/packages/rmw-shell/cra-template-rmw/template/src/index.js#L19

The https://pawll.glitch.me/ presentation explains why calling skipWaiting() when you're also using lazy-loaded, versioned, precached resources is a bad idea.

@TarikHuber
Copy link
Author

TarikHuber commented Nov 12, 2020

omg such a stupid bug 😢

It should be a ()=>reg.waiting.postMessage({ type: 'SKIP_WAITING' }) and not reg.waiting.postMessage({ type: 'SKIP_WAITING' })

Thank you so much for the fast response and help with this! 😄

TarikHuber added a commit to ecronix/react-most-wanted that referenced this issue Nov 12, 2020
PhearZero pushed a commit to React-Junkie/rmw-shell that referenced this issue Mar 7, 2021
masterciel added a commit to masterciel/react_most_wanted that referenced this issue Mar 12, 2023
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

No branches or pull requests

2 participants