chore(CRWA): convert to ESM #8159

jtoar · 2023-04-28T05:52:01Z

Edit: I've since added a .babelrc.js file for testing in a separate PR so the reasons for this PR detailed above aren't as valid. This package being a suitable build-tool playground is still pretty valid though.

I want to add more unit tests to CRWA, but since I've removed Babel from the equation, I've also removed Jest. I could add back the .babelrc.js file, but CRWA is a really nice package to try things out in since its 1) a binary like the CLI, not a library (so it's never imported from) and 2) uses very few other @redwoodjs/* as dependencies.

So I want to try out writing tests using the new node test runner, and to do that in a nice way, this package needs to be ESM so that I can just import the functions straight from src.

I manually published the changes in this PR under a different package name (and then deleted that pacakge) just to try it out, and it worked using both yarn 1 and yarn 3. After merge we can confirm again via canary, then I could get started on more unit tests.

jtoar · 2023-04-28T05:53:28Z

packages/create-redwood-app/package.json

-    "build:watch": "nodemon --watch src --ignore dist,template --exec \"yarn build\"",
-    "prepublishOnly": "NODE_ENV=production yarn build"
+    "test": "yarn node --test"


esbuild sets NODE_ENV to production if minify is true, and here it is. See https://esbuild.github.io/api/#platform. (But more generally, I'm sure NODE_ENV really does anything here.)

jtoar · 2023-04-28T05:56:56Z

packages/create-redwood-app/src/create-redwood-app.js

@@ -152,7 +161,7 @@ async function executeCompatibilityCheck(templateDir, yarnInstall) {
 */
 function checkNodeAndYarnVersion(templateDir) {
  return new Promise((resolve) => {
-    const { engines } = require(path.join(templateDir, 'package.json'))
+    const { engines } = fs.readJSONSync(path.join(templateDir, 'package.json'))


To use require now, we'd have to create it using createRequire.

jtoar · 2023-04-28T05:59:11Z

packages/create-redwood-app/tests/template.test.js

+// If you add, move, or remove a file from the create-redwood-app template,
+// you'll have to update this test.


I thought snapshots were added to Node's test runner (or assert module), but I couldn't find them, so maybe I misread or they've been pulled out since

Seems like they were added here, but I can't find any docs on them: nodejs/node#44095

This comment here hints that it was removed, but still can't find the commit: nodejs/node#47392 (comment)

Found nodejs/node#46112 with it's linked comment

replay-io · 2023-04-28T06:11:16Z

19 replays were recorded for aec1bfd.

0 Failed

19 Passed

requireAuth graphql checks

locator.waitFor: Target closed
=========================== logs ===========================
waiting for locator('.rw-form-error-title').locator('text=You don\'t have permission to do that') to be visible
============================================================

useAuth hook, auth redirects checks
Check that a specific blog post is prerendered
Check that about is prerendered
Check that homepage is prerendered
Check that meta-tags are rendering the correct dynamic data
Check that rehydration works for page not wrapped in Set
Check that rehydration works for page with Cell in Set
Check that rehydration works for page with code split chunks
Check that you can navigate from home page to specific blog post
Waterfall prerendering (nested cells)
RBAC: Admin user should be able to delete contacts
RBAC: Should not be able to delete contact as non-admin user
Smoke test with dev server
Smoke test with rw serve
Loads Cell mocks when Cell is nested in another story
Loads Cell Stories
Loads MDX Stories
Mocks current user, and updates UI while dev server is running

View test run on Replay ↗︎

replay-io · 2023-04-28T06:11:17Z

16 replays were recorded for 1ff7499.

0 Failed

16 Passed

requireAuth graphql checks

    ```
    locator.waitFor: Target closed
    =========================== logs ===========================
    waiting for locator('.rw-form-error-title').locator('text=You don\'t have permission to do that') to be visible
    ============================================================
    ```
  </ol>
</details>
<li><a href=https://app.replay.io/recording/660d5f57-91e9-4f9e-9af0-51e3cc682bd4>useAuth hook, auth redirects checks</a></li>
<li><a href=https://app.replay.io/recording/bf68be4a-20ed-4596-b91b-976d7b4b3894>Check that a specific blog post is prerendered</a></li>
<li><a href=https://app.replay.io/recording/03b317c4-7e0b-4747-bcf0-fa2285c057f9>Check that about is prerendered</a></li>
<li><a href=https://app.replay.io/recording/b64af903-ec66-4670-b026-8514fcb19bbe>Check that homepage is prerendered</a></li>
<li><a href=https://app.replay.io/recording/f6fc880b-c37e-4d55-9b4f-68acf371a6df>Check that meta-tags are rendering the correct dynamic data</a></li>
<li><a href=https://app.replay.io/recording/ef2f2029-9ca6-4d22-b15a-4f23d64f4ae0>Check that you can navigate from home page to specific blog post</a></li>
<li><a href=https://app.replay.io/recording/0dedf861-f72f-44bd-8c99-9cad6a92e35f>Waterfall prerendering (nested cells)</a></li>
<li><a href=https://app.replay.io/recording/4276ac32-9b46-453c-86a1-6aa83a0fc6e9>RBAC: Admin user should be able to delete contacts</a></li>
<li><a href=https://app.replay.io/recording/54c5fb2e-a513-4cf3-a020-8e58b056c9cf>RBAC: Should not be able to delete contact as non-admin user</a></li>
<li><a href=https://app.replay.io/recording/84c57c02-5db9-42d8-9ce3-b64c2d00dbef>Smoke test with dev server</a></li>
<li><a href=https://app.replay.io/recording/bab052db-175e-4ed9-824c-57f29c5958b7>Smoke test with rw serve</a></li>
<li><a href=https://app.replay.io/recording/121b4f6b-ecac-40c6-9078-a5d841c005ad>Loads Cell mocks when Cell is nested in another story</a></li>
<li><a href=https://app.replay.io/recording/2c52c860-a63c-4e7f-95eb-d83d9702e5b4>Loads Cell Stories</a></li>
<li><a href=https://app.replay.io/recording/d73f2356-6703-49b7-b84a-ebef49808636>Loads MDX Stories</a></li>
<li><a href=https://app.replay.io/recording/9172c2b8-896b-4911-893a-77627f59e22f>Mocks current user, and updates UI while dev server is running</a></li>

View test run on Replay ↗︎

jtoar · 2023-04-28T06:27:18Z

packages/create-redwood-app/package.json

@@ -14,8 +15,7 @@
  ],
  "scripts": {
    "build": "yarn node ./build.mjs",
-    "build:watch": "nodemon --watch src --ignore dist,template --exec \"yarn build\"",
-    "prepublishOnly": "NODE_ENV=production yarn build"
+    "test": "yarn node ./tests/template.test.js"


I tried yarn node --test at first, but when we run yarn test from the root, this command fails because it's fed bad options (jest related ones, like colors and maxWorkers). Something to figure out, but for now this works

packages/create-redwood-app/tests/template.test.js

Josh-Walker-GM · 2023-04-28T12:29:38Z

I changed the esbuild logLevel to info from it's default of warning. Docs here https://esbuild.github.io/api/#log-level but I like info more as it's the default value when used with the cli and it shows at least a little success message when running yarn build from within the package.

Josh-Walker-GM · 2023-04-28T12:57:49Z

packages/create-redwood-app/package.json

@@ -14,8 +15,7 @@
  ],
  "scripts": {
    "build": "yarn node ./build.mjs",
-    "build:watch": "nodemon --watch src --ignore dist,template --exec \"yarn build\"",
-    "prepublishOnly": "NODE_ENV=production yarn build"
+    "test": "yarn node --test-reporter spec ./tests/template.test.js"


I set --test-reporter spec because I was getting the tap reporter on my vscode terminal. I don't really think it'll make that much difference to switch it as we're not passing these test logs into anything programmatic?

https://nodejs.org/api/test.html#test-reporters

Josh-Walker-GM

We can tidy up things like the yarn test command later as things evolve and become clearer. I don't think it holds this back.

Tobbe · 2023-04-28T17:09:26Z

packages/create-redwood-app/src/telemetry.js

 import system from 'systeminformation'

-import { name as packageName, version as packageVersion } from '../package'
+// JSON modules aren't stable yet, but if they were we could use them instead.
+// See https://nodejs.org/dist/latest-v18.x/docs/api/esm.html#json-modules.


This was interesting to me, so I did some digging.
It's still experimental even in Node 20: https://nodejs.org/docs/v20.0.0/api/esm.html#json-modules

It looks like it'll change syntax slightly before it fully ships (assert will change to with)
https://github.com/tc39/proposal-import-attributes
Scrolling to the very bottom there's an interesting "History" section. That section also mentions that compatibility with assert will probably remain even after the official syntax switches to with

TIL 🙂

jtoar · 2023-07-14T19:59:08Z

This was mostly an experiment; we learned a lot. This will still probably happen one way or another eventually (along with the rest of the packages) but this doesn't need to stay open at the moment.

see: redwoodjs#8159

jtoar added the release:chore This PR is a chore (means nothing for users) label Apr 28, 2023

jtoar requested a review from Josh-Walker-GM April 28, 2023 05:52

jtoar commented Apr 28, 2023

View reviewed changes

jtoar force-pushed the ds-crwa/esm-and-tests branch 2 times, most recently from 6079e3b to 9f61b19 Compare April 28, 2023 05:55

jtoar commented Apr 28, 2023

View reviewed changes

jtoar force-pushed the ds-crwa/esm-and-tests branch from 9f61b19 to 1ff7499 Compare April 28, 2023 05:58

jtoar commented Apr 28, 2023

View reviewed changes

jtoar force-pushed the ds-crwa/esm-and-tests branch from 1ff7499 to ac3849d Compare April 28, 2023 06:12

change to esm and add a template test

2cdba03

jtoar force-pushed the ds-crwa/esm-and-tests branch from ac3849d to 2cdba03 Compare April 28, 2023 06:25

jtoar commented Apr 28, 2023

View reviewed changes

packages/create-redwood-app/tests/template.test.js Outdated Show resolved Hide resolved

Josh-Walker-GM added 2 commits April 28, 2023 13:15

Fix test failure on windows

b2034b4

Move to 'info' logLevel for esbuild

815faba

Josh-Walker-GM added 2 commits April 28, 2023 13:39

lint fix

9338b02

Set --test-reporter spec

098e523

Josh-Walker-GM reviewed Apr 28, 2023

View reviewed changes

Josh-Walker-GM approved these changes Apr 28, 2023

View reviewed changes

Tobbe reviewed Apr 28, 2023

View reviewed changes

Merge branch 'main' into ds-crwa/esm-and-tests

dc68e6b

jtoar changed the title ~~chore(CRWA): convert to ESM and add a template test~~ chore(CRWA): convert to ESM May 5, 2023

jtoar marked this pull request as draft May 5, 2023 03:22

Merge branch 'main' into ds-crwa/esm-and-tests

aec1bfd

jtoar marked this pull request as ready for review May 19, 2023 00:52

jtoar marked this pull request as draft May 19, 2023 01:10

jtoar closed this Jul 14, 2023

jtoar deleted the ds-crwa/esm-and-tests branch July 28, 2023 04:26

virtuoushub added a commit to virtuoushub/redwood that referenced this pull request Dec 12, 2023

chore(CRWA): convert to ESM

29ec2bd

see: redwoodjs#8159

virtuoushub added a commit to virtuoushub/redwood that referenced this pull request Dec 12, 2023

chore(CRWA): convert to ESM

7ebf849

see: redwoodjs#8159

virtuoushub mentioned this pull request Dec 12, 2023

chore(crwa): Convert Create Redwood App to ESM #9659

Closed

virtuoushub added a commit to virtuoushub/redwood that referenced this pull request Dec 12, 2023

chore(CRWA): convert to ESM

10a2600

see: redwoodjs#8159

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore(CRWA): convert to ESM #8159

chore(CRWA): convert to ESM #8159

jtoar commented Apr 28, 2023 •

edited

jtoar Apr 28, 2023

jtoar Apr 28, 2023

jtoar Apr 28, 2023

jtoar Apr 28, 2023

jtoar Apr 28, 2023

Josh-Walker-GM Apr 28, 2023

replay-io bot commented Apr 28, 2023 •

edited

replay-io bot commented Apr 28, 2023

jtoar Apr 28, 2023

Josh-Walker-GM commented Apr 28, 2023

Josh-Walker-GM Apr 28, 2023

Josh-Walker-GM left a comment

Tobbe Apr 28, 2023

jtoar commented Jul 14, 2023

		// If you add, move, or remove a file from the create-redwood-app template,
		// you'll have to update this test.

chore(CRWA): convert to ESM #8159

chore(CRWA): convert to ESM #8159

Conversation

jtoar commented Apr 28, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

replay-io bot commented Apr 28, 2023 • edited

replay-io bot commented Apr 28, 2023

Choose a reason for hiding this comment

Josh-Walker-GM commented Apr 28, 2023

Choose a reason for hiding this comment

Josh-Walker-GM left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jtoar commented Jul 14, 2023

jtoar commented Apr 28, 2023 •

edited

replay-io bot commented Apr 28, 2023 •

edited