feat: day display variant

[TobiTenno]

the default value for the variant is simple, i.e. the original for this example app

this should look familiar, as it's based on https://romcal.js.org

[tukusejssirs]

@TobiTenno, how do you run it? It fails to build on my end. :thinking:

---

pnpm start throws some errors (always using nvm use in the parent folder). The build (in pnpm start, i.e. until I see something output on the website) takes for about two minutes.

pnpm start errors

```bash # rm -rf package-lock.json pnpm-lock.yaml node_modules && git checkout yarn.lock && pnpm i && pnpm start webpack compiled successfully ERROR in src/components/LocaleMenu.tsx:4:10 TS2614: Module '"romcal"' has no exported member 'Romcal'. Did you mean to use 'import Romcal from "romcal"' instead? 2 | import { observer } from 'mobx-react'; 3 | import React, { useContext } from 'react'; > 4 | import { Romcal } from 'romcal'; | ^^^^^^ 5 | import { AppContext } from '../AppContext'; 6 | 7 | const LocaleMenu = observer(() => { ERROR in src/components/LocaleMenu.tsx:14:77 TS7006: Parameter 'pkg' implicitly has an 'any' type. 12 | const { localeId, setLocaleId } = romcalStore; 13 | > 14 | const allLocales = Romcal.LOCALE_IDS.reduce((acc: Record, pkg, index) => { | ^^^ 15 | acc[Romcal.LOCALE_VAR_NAMES[index]] = pkg; 16 | return acc; 17 | }, {}); ERROR in src/components/LocaleMenu.tsx:14:82 TS7006: Parameter 'index' implicitly has an 'any' type. 12 | const { localeId, setLocaleId } = romcalStore; 13 | > 14 | const allLocales = Romcal.LOCALE_IDS.reduce((acc: Record, pkg, index) => { | ^^^^^ 15 | acc[Romcal.LOCALE_VAR_NAMES[index]] = pkg; 16 | return acc; 17 | }, {}); ERROR in src/components/LocaleMenu.tsx:32:15 TS2322: Type 'unknown' is not assignable to type 'ReactNode'. 30 | {Object.entries(allLocales).map(([id, isoId]) => ( 31 | > 32 | {isoId} | ^^^^^^^ 33 | 34 | ))} 35 | ERROR in src/RomcalApp.test.tsx:8:23 TS2339: Property 'toBeInTheDocument' does not exist on type 'JestMatchers'. 6 | render(); 7 | const textElement = screen.getByText(/A JavaScript library/i); > 8 | expect(textElement).toBeInTheDocument(); | ^^^^^^^^^^^^^^^^^ 9 | }); 10 | ERROR in src/stores/romcal.store.ts:3:10 TS2614: Module '"romcal"' has no exported member 'Romcal'. Did you mean to use 'import Romcal from "romcal"' instead? 1 | import { addMonths, subMonths } from 'date-fns'; 2 | import { makeAutoObservable, runInAction } from 'mobx'; > 3 | import { Romcal, BaseLiturgicalDay } from 'romcal'; | ^^^^^^ 4 | import { CALENDARS } from '../constants/calendars'; 5 | 6 | export class RomcalStore { ```

---

It works with NPM (npm start), whoever, there are some warning output in the terminal, but the website loads, albeit after some long time (which might be an expected side effect of this PR). It look about 1.5 minutes to load the website, while the 4-8 seconds were needed to actually load the website after Webpack packed the code.

npm start warnings

```bash # rm -rf package-lock.json pnpm-lock.yaml node_modules && git checkout yarn.lock && npm i && npm start Compiled with warnings. Failed to parse source map from '$repo_root/react-app/node_modules/html-entities/src/index.ts' file: Error: ENOENT: no such file or directory, open '$repo_root/react-app/node_modules/html-entities/src/index.ts' Failed to parse source map from '$repo_root/react-app/node_modules/html-entities/src/named-references.ts' file: Error: ENOENT: no such file or directory, open '$repo_root/react-app/node_modules/html-entities/src/named-references.ts' Failed to parse source map from '$repo_root/react-app/node_modules/html-entities/src/numeric-unicode-map.ts' file: Error: ENOENT: no such file or directory, open '$repo_root/react-app/node_modules/html-entities/src/numeric-unicode-map.ts' Failed to parse source map from '$repo_root/react-app/node_modules/html-entities/src/surrogate-pairs.ts' file: Error: ENOENT: no such file or directory, open '$repo_root/react-app/node_modules/html-entities/src/surrogate-pairs.ts' Search for the keywords to learn more about each warning. To ignore, add // eslint-disable-next-line to the line before. WARNING in ./node_modules/html-entities/lib/index.js Module Warning (from ./node_modules/source-map-loader/dist/cjs.js): Failed to parse source map from '$repo_root/react-app/node_modules/html-entities/src/index.ts' file: Error: ENOENT: no such file or directory, open '$repo_root/react-app/node_modules/html-entities/src/index.ts' WARNING in ./node_modules/html-entities/lib/named-references.js Module Warning (from ./node_modules/source-map-loader/dist/cjs.js): Failed to parse source map from '$repo_root/react-app/node_modules/html-entities/src/named-references.ts' file: Error: ENOENT: no such file or directory, open '$repo_root/react-app/node_modules/html-entities/src/named-references.ts' WARNING in ./node_modules/html-entities/lib/numeric-unicode-map.js Module Warning (from ./node_modules/source-map-loader/dist/cjs.js): Failed to parse source map from '$repo_root/react-app/node_modules/html-entities/src/numeric-unicode-map.ts' file: Error: ENOENT: no such file or directory, open '$repo_root/react-app/node_modules/html-entities/src/numeric-unicode-map.ts' WARNING in ./node_modules/html-entities/lib/surrogate-pairs.js Module Warning (from ./node_modules/source-map-loader/dist/cjs.js): Failed to parse source map from '$repo_root/react-app/node_modules/html-entities/src/surrogate-pairs.ts' file: Error: ENOENT: no such file or directory, open '$repo_root/react-app/node_modules/html-entities/src/surrogate-pairs.ts' webpack compiled with 4 warnings ```

---

Also, we might want to upgrade the romcal packages to v3.0.0-dev.60.

---

Consider committing package-lock.json. I am not sure though whether we should commit package.json, yarn.lock and pnpm-lock.yaml at the same time and if we do so, how it would behave. :thinking: Nevertheless, npm i updates yarn.lock and creates package-lock.json. Running pnpm i only creates pnpm-lock.yaml.

[TobiTenno]

@tukusejssirs

how do you run it?

same way as in the readme. I've got warnings on build, but i've always had a slow startup on the website part, but it usually only takes 10-15 seconds after initial server start for the dev server to cache all the assets, it's probably related to loading up all the calendars.

edit: after switching to package-lock, the load-up of the calendars seems to be much faster, or tangential, to the startup speed. still looking into the warnings

committing package-lock.json

I've committed the updated yarn.lock, but i didn't want to switch to package-lock.json without consensus, likewise on updating to updating the dev packages. I'm hoping to keep changes in digestible chunks, but i'm happy to do some of these extra if they help out.

pnpm start errors

i'm wondering if those are related to switching a good bit of the processing to ESM-native versions, as that will probably change package resolution, which i'm not sure how pnpm handles those (thus sticking w/ npm)

npm start warnings

I'm seeing the same warnings, but i was seeing similar from just running dependency updates, so they look related to dependencies. i'll see if i can narrow down precisely where they come from, but from the research i'd seen, these are just react-app dependencies, so unfortunately part of trying to stay up-to-date on upstream dependencies

[TobiTenno]

npm warnings

Overriding the version of html-entities to the latest seems to remedy this issue. I use an override rather than including it as a dependency because this allows us to not include more in our manifest, just to force our dependencies to use the same good version uniformly.

[tukusejssirs]

I've committed the updated yarn.lock, but i didn't want to switch to package-lock.json without consensus, likewise on updating to updating the dev packages.

FYI: As far as I remember it correctly, Etienne wanted to switch to PNPM in the monorepo.

I personally prefer using PNPM, while I test it with NPM too. I use Yarn only when I see its lock file committed or it is hard-coded somewhere (like in an NPM script).

I was thinking if it would cause some issues if we commit all three lock files. :thinking: I have never done that though.

I'm hoping to keep changes in digestible chunks, but i'm happy to do some of these extra if they help out.

It might be a good idea to move the lock files discussion out of this PR, however, I think we should talk about it.

Overriding the version of html-entities to the latest seems to remedy this issue. I use an override rather than including it as a dependency because this allows us to not include more in our manifest, just to force our dependencies to use the same good version uniformly.

How do you override it? Do you need to patch something up?

[TobiTenno]

@tukusejssirs

How do you override it? Do you need to patch something up?

the overrides field in the package.json

[tukusejssirs]

I still get the error described in here when using PNPM (it does not happen with NPM). I have no idea why would a test be run when starting the app. :thinking:

I’d like to debug this, hopefully later this week.

[TobiTenno]

I still get the error described in here when using PNPM (it does not happen with NPM). I have no idea why would a test be run when starting the app. 🤔

I’d like to debug this, hopefully later this week.

I'll spin up pnpm to see if i can reproduce it as well, but it's not a test, but i'm not sure either. maybe test is a default in pnpm for pre-start? I'm not entirely sure as it could be related to react-scripts, which i was going to promote avoiding, but makes sense in this specific case for the purposes of an example app being as close to CRA (create-react-app) as possible

[tukusejssirs]

Well, src/RomcalApp.test.tsx seems to be a test and it errors out on expect(textElement).toBeInTheDocument().

maybe test is a default in pnpm for pre-start?

There are no default pre* scripts in PNPM. Actually, without enabling enable-pre-post-scripts=true (e.g. in .npmrc), no pre* scripts are run when using PNPM.

[TobiTenno]

yeah, i know about the parallel test files, but i'll be honest, i haven't touched them much yet. I'll dig through that once I finish some of the linting QOL on the main repo

[TobiTenno]

looking at the error, looks like pnpm start (i still don't know why, this part i'm not terribly worried about) is running tests that default npm start doesn't run. That test was failing, i've now fixed said test.

that said, startup with pnpm took significantly longer... I wish i had a timer for this for how long it took to get to first render, but i didn't wanna spend the time doing it.

interestingly, concerning pnpm,
if i npm install and then pnpm start, this works 100%, and loads fast. if i pnpm install and pnpm start, this fails, and seems to miss the function existing because, by default, the npm version sees or sets up the tests w/ setupTests.ts before trying to compile the test file... it's just weird

ok, so i found that the RomcalApp.test.tsx was attempting to get compiled as part of the app... this was weird to me, so i moved it to a tests folder (iirc, this is one of the standard jest locations), and while the test if failing (still need to check this part), the start can work successfully without excess work.

i still need to go make a new jest config that works with ts & esm at the same time, but the rest of the app starts fine with it

[TobiTenno]

@tukusejssirs is there anything else i can fix before you're ok with the example updates?

[tukusejssirs]

I think the long website startup when using PNPM is caused by the fact that all calendar chunks (e.g. /static/js/vendors-node_modules_pnpm_romcal_calendar_argentina_3_0_0-dev_60_romcal_3_0_0-dev_60_node_mod-204aff.chunk.js). IMHO the UI should be loaded ASAP, then the data for the default calendar (the GRC) in the default locale (en) should be loaded, the rest of the data should be lazily downloaded after the website is fully loaded from the user/viewer perspective.

However, that might be a task for a separate PR. Note that I am not a frontend dev (I know a bit of HTML and CSS, but never used React, but I used a bit Svelte and Vite 3), therefore, I am not sure if or how to carry out what I suggested above.

---

I think you should upgrade romcal deps to their latest version (3.0.0-dev.67). This is quite important IMO.

---

This is probably unrelated to this PR, however, the test fails (both using NPM and PNPM):

 FAIL  src/__tests__/RomcalApp.test.tsx
  ● Test suite failed to run

    Jest encountered an unexpected token

    Jest failed to parse a file. This happens e.g. when your code or its dependencies use non-standard JavaScript syntax, or when Jest is not configured to support such syntax.

    Out of the box Jest supports Babel, which will be used to transform your files into valid JS based on your Babel configuration.

    By default "node_modules" folder is ignored by transformers.

    Here's what you can do:
     • If you are trying to use ECMAScript Modules, see https://jestjs.io/docs/ecmascript-modules for how to enable it.
     • If you are trying to use TypeScript, see https://jestjs.io/docs/getting-started#using-typescript
     • To have some of your "node_modules" files transformed, you can specify a custom "transformIgnorePatterns" in your config.
     • If you need a custom transformation specify a "transform" option in your config.
     • If you simply want to mock your non-JS modules (e.g. binary assets) you can stub them out with the "moduleNameMapper" config option.

    You'll find more details and examples of these config options in the docs:
    https://jestjs.io/docs/configuration
    For information about custom transformations, see:
    https://jestjs.io/docs/code-transformation

    Details:

    /run/media/ts/root/home/ts/git/c10n/romcal-examples__TobiTenno/react-app/src/constants/calendars.ts:10
      GeneralRoman: await Promise.resolve().then(() => (0, _interopRequireWildcard2.default)(require('@romcal/calendar.general-roman'))),
                          ^^^^^^^

    SyntaxError: Unexpected identifier 'Promise'

      2 | import { makeAutoObservable, runInAction } from 'mobx';
      3 | import { Romcal, BaseLiturgicalDay } from 'romcal';
    > 4 | import { CALENDARS } from '../constants/calendars';
        | ^
      5 |
      6 | export class RomcalStore {
      7 |   fetchingData: boolean = false;

      at Runtime.createScriptFromCode (node_modules/.pnpm/jest-runtime@27.5.1/node_modules/jest-runtime/build/index.js:1728:14)
      at Object.<anonymous> (src/stores/romcal.store.ts:4:1)
      at Object.<anonymous> (src/stores/index.ts:1:1)
      at Object.<anonymous> (src/AppContext.tsx:2:1)
      at Object.<anonymous> (src/RomcalApp.tsx:6:1)
      at Object.<anonymous> (src/__tests__/RomcalApp.test.tsx:4:1)
      at TestScheduler.scheduleTests (node_modules/.pnpm/@jest+core@27.5.1/node_modules/@jest/core/build/TestScheduler.js:333:13)
      at runJest (node_modules/.pnpm/@jest+core@27.5.1/node_modules/@jest/core/build/runJest.js:404:19)

---

In the end, if you consider this PR done, go ahead and merge it.

[TobiTenno]

re: Tests i'm not targeting the tests, because they didn't do anything before either, and the issue with jest is jest just doesn't work, so i'm trying to avoid ejecting react-scripts and having to kill of jest entirely. this is essentially a "why jest is bad" case.

---

re: new versions this was started long before the newer versions were out. i'll happily do that, but that's well out of scope of the day display variant. I'm happy to do another PR, but i'm not cool leaving PRs to sit for months because we want to keep upgrading an individual PR. if it was something blocking this day variant from working entirely due to the version, I'd agree

---

re: pnpm

yes, that's related to awaiting the construction of them all. I'm happy to revert that part that I added, as that's directly related to my changes and somewhat added.

the startup time is longer for first launch with my current (awaiting them all in CALENDARS), but it's still long on the startup without the current state. i'll see if i can move some of the initial startup waits off of the main thread entirely for getting react to turn stuff on

[tukusejssirs]

tests & jest is a bad tool for the use case

Yeah, Jest is for unit tests (and we should use it for that), however, for UI tests we might want to use Playwright (in a separate PR, OFC).

new romcal versions

Okay, then do it in a separate PR please. There some imporant bug fixes (#477, #480), however, it is not important from POV of this app.

the startup time is longer for first launch with my current (awaiting them all in CALENDARS), but it's still long on the startup without the current state.

Subjectively, I feel it is a bit faster than it was before this PR. :man_shrugging:

i'll see if i can move some of the initial startup waits off of the main thread entirely for getting react to turn stuff on

If you want, you can do this in a separate PR. IMHO the order of loading priority is as follows:

1. UI;
2. the default calendar;
3. the default locale;
4. all the rest.

[tukusejssirs]

Sorry for the previous comment; I have published more than intended. I have updated the comment to contain only what is related to this PR.

[TobiTenno]

re: speed

yeah, the problem, from what i'm seeing, is that the async imports make it pretty slow. i'm going to try to speed it up a bit, cause i'm not sure why it needs to be awaited at all, so i'm working to optimize the speed a good bit more.

---

i'd definitely like to add better ui testing, because the implementation of how jest works for UI testing is just.... bad.

I'll probably do cypress, but i'll happily take a look at playwright.

---

just to summarize,

• in the current PR:
  • fix load speed related to current changes
• in upcoming prs:
  • update romcal version for all examples (#3)
  • add UI testing to replace jest "unit" testing, because jest doesn't work anymore for what we need (#4)

if there's anything else, i'd like to make a small set of issues for known stuff to do and i'll link them to this one

[tukusejssirs]

yeah, the problem, from what i'm seeing, is that the async imports make it pretty slow. i'm going to try to speed it up a bit, cause i'm not sure why it needs to be awaited at all, so i'm working to optimize the speed a good bit more.

:+1: :pray:

i'd definitely like to add better ui testing, because the implementation of how jest works for UI testing is just.... bad.

Yeah, it is not the right tool for the task.

I'll probably do cypress, but i'll happily take a look at playwright.

Use whatever you want. Either is fine with me, I have just read that Playwright might be better/preferred these days, however, I am not a frontend dev. :man_shrugging:

just to summarize,

• in the current PR:
  • fix load speed related to current changes
• in upcoming prs:
  • update romcal version for all examples (#3)
  • add UI testing to replace jest "unit" testing, because jest doesn't work anymore for what we need (#4)

if there's anything else, i'd like to make a small set of issues for known stuff to do and i'll link them to this one

I agree with this plan, however, I do think that for actual unit tests (e.g. functions, methods), Jest does a good job, however, not for UI tests.

Thanks! :pray: