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

[WIP] Features/vite plays outside updated json/578 #1253

Closed
wants to merge 16 commits into from

Conversation

Angra974
Copy link
Contributor

First thing, PLEASE READ THIS: ReactPlay Code Review Checklist

WIP : This is not made in the intend to be merged but a showcase

Description

  • Use vite as a bundler
  • WPA ready
  • use picture for thumbnail instead of image tag when possible
    • if image is generated and information is present in index.json file

      • image will be loaded directly from the disk with the right extension
    • else

      • image will be handled by loadCoverImage
    • else

      • looking for an external provider for this cover
        - else
      • load the default thumb
    • plays can be inside src/plays folder or be in another folder

      • folder path need to be valid in tsconfig json and vite.config.js
    • package.json file contains only modules needed for the main app to run.
      developer need to add package with the "-w" tag to be use in the workspace

    • each play has it own package.json file. This package.json file is created by create-react-play #43
      => developer need to add package in their play when they create the play by using
      "pnpm add my_package"

    • Typescript version is set

    • Eslint give no error for configuration

    • Only allow pnpm - no other builder will be used in this app

    • husky prepush with cypress test
      => push are done only if test doesn't fail
      => test was rewritten to pass the current build

    • index.json file
      => create by create-react-play [✨ [Feature request]: Show no play found when there is no match for filter and search result  #43]
      => contains the minium information needed to run react-play and plays
      => a relative path to the play folder. Need for import with vitejs as relative path to file.
      => contains play slug name and play name so it's easy to recreate the path to the file
      => used to load all plays in playlist.jsx as a filter to the request but load only the required information and not all the plays are imported as it's actually done.
      => use as a filter in playMeta.jsx to import only one play in the require page and not all plays as it's actually done
      => use to handle navigation between play in the individual play page. So, developer can have a previous and next button to click to test if their modification affect other play or not. I use it to test which play needed a personnal package or not when created individual package.json file

    • github workflow rewritten to use pnpm

    Tested are run with Vitest and made compatible with Jest so all the current test are valid and pass the tests.

Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.

This is not intended to be merged but to show how index.json file can be use for improving the performance of the application and code organisation for using plays outiside the main application and vite.

Fixes #1251
Fixes #1229
Fixes #1243
Fixes #1239
Fixes #1217
Fixes #1212
Fixes #1193
Fixes #1189
Fixes #1100
Fixes #866

Know issues

Didn't test play created by the developer and how to handle it in playList.js has it was not my intention to do it.
The playList.jsx was recoded only to be able do display the content from index.json so not all functionnalities in playList.jsx was recreated and handled correctly. Will need more attention later.

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

Test was done with vitest for component test
Tests was done with cypress for e2e and pages load
All pages was tested
All plays were controlled individually and rewritten when necessary and minimal package installed to get them running well.
prepush and test before prepush was tested when making this request

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce.

Checklist:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • [] I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

Screenshots or example output

image

image

image

@vercel
Copy link

vercel bot commented Sep 25, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
react-play ❌ Failed (Inspect) Sep 26, 2023 2:23am

@netlify
Copy link

netlify bot commented Sep 25, 2023

Deploy Preview for reactplayio failed.

Name Link
🔨 Latest commit 19c8a8c
🔍 Latest deploy log https://app.netlify.com/sites/reactplayio/deploys/6512406589a5c80008c93830

@Angra974 Angra974 added work in progress The work is in progress idea testing labels Sep 25, 2023
@joshi-kaushal
Copy link
Member

Hi @Angra974 I want an update on this. need to plan further things accordingly

Copy link

github-actions bot commented Nov 7, 2023

There hasn't been any activity on this pull request recently, and in order to prioritize active work, it has been marked as stale. This PR will be closed and locked in 7 days if no further activity occurs. Thank you for your contributions!

@github-actions github-actions bot added the stale label Nov 7, 2023
@github-actions github-actions bot closed this Nov 14, 2023
@priyankarpal priyankarpal deleted the features/vite_plays-outside-updatedJson/578 branch December 27, 2023 03:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment