search

processing / p5.js-web-editor

The p5.js Editor is a website for creating p5.js sketches, with a focus on making coding accessible and inclusive for artists, designers, educators, beginners, and anyone else! You can create, share, or remix p5.js sketches without needing to download or configure anything.
https://editor.p5js.org
GNU Lesser General Public License v2.1
1.34k stars 1.29k forks source link

Assets Page Not Loading #2889

Closed raclim closed 7 months ago

raclim commented 7 months ago

p5.js version

1.9.0

What is your operating system?

Mac OS

Current Behavior

Currently, when a user attempts to access their assets, they're led to a blank page. The error that pops up in the console for this page is "TypeError: cannot read properties of undefined (reading 'length')" in AssetList.jsx (pictured below).

screenshot picturing the typerror described above

This might be due to the asset state values (list and totalSize) not being set properly within client/modules/IDE/actions and client/modules/IDE/reducers.

Edit: I implemented a quick fix for it in https://github.com/processing/p5.js-web-editor/commit/71984a96abc915db201f0834219759ec7b7f4c60, but also open to any other solutions for this.

lindapaiste commented 7 months ago

@raclim Your fix is fine.

I think in the the future I need to be the one to fix merge conflicts in my PRs. Let me know if there is one that you are ready to review and I can make sure that it's up to date. I appreciate you putting in the effort but what happened here was due to merging the conflicts improperly. See https://github.com/processing/p5.js-web-editor/commit/b422eb68c06ca160b81a4c8e7d023ed0f46ef704#diff-66d6c96212508701969243bbfdbe0a00f80b90ad538b4970dbb5ec5468dcb6b0 for the differences before and after merge.

In my original PR https://github.com/processing/p5.js-web-editor/pull/2248/commits/1601e0ff0eab97d30b0da273e2689aa944471447 I changed the line in client/modules/IDE/actions/assets.js from

dispatch(setAssets(response.data.assets, response.data.totalSize));

to

dispatch(
  setAssets({
    list: response.data.assets,
    totalSize: response.data.totalSize
  })
);

where the payload of the action matches the state of the reducer and therefore we can set the payload as the new state

setAssets: (state, action) => action.payload

When you resolved the conflicts you inadvertently overwrote that part of the code changes and it went back to dispatch(setAssets(response.data.assets, response.data.totalSize));, which doesn't work with the new action creator.

The longer these PRs sit, the more conflicts pop up. The PR #2436 which created the conflict was submitted 3 months after I wrote this code.

raclim commented 7 months ago

@lindapaiste Thanks for letting me know. I figured it was probably something that happened either on my end or from the PRs sitting for a while since there were a lot of changes. I'll leave the merge conflicts in your PRs from now on.

I'll place any upcoming PRs in the Milestones. I've been trying to start going through PRs chronologically from oldest -> latest, so the older ones will most likely be up next in the near future. Also, feel free to to add any in there as well that you want to prioritize.