Conversation
| "build:watch": "nodemon --watch src --ignore dist,template --exec \"yarn build\"", | ||
| "prepublishOnly": "NODE_ENV=production yarn build" | ||
| "test": "yarn node --test" |
There was a problem hiding this comment.
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.)
6079e3b to
9f61b19
Compare
| function checkNodeAndYarnVersion(templateDir) { | ||
| return new Promise((resolve) => { | ||
| const { engines } = require(path.join(templateDir, 'package.json')) | ||
| const { engines } = fs.readJSONSync(path.join(templateDir, 'package.json')) |
There was a problem hiding this comment.
To use require now, we'd have to create it using createRequire.
9f61b19 to
1ff7499
Compare
| // If you add, move, or remove a file from the create-redwood-app template, | ||
| // you'll have to update this test. |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
Seems like they were added here, but I can't find any docs on them: nodejs/node#44095
There was a problem hiding this comment.
This comment here hints that it was removed, but still can't find the commit: nodejs/node#47392 (comment)
There was a problem hiding this comment.
Found nodejs/node#46112 with it's linked comment
|
16 replays were recorded for 1ff7499.
|
1ff7499 to
ac3849d
Compare
ac3849d to
2cdba03
Compare
| "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" |
There was a problem hiding this comment.
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
|
I changed the esbuild |
| "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" |
There was a problem hiding this comment.
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?
Josh-Walker-GM
left a comment
There was a problem hiding this comment.
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.
|
|
||
| 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. |
There was a problem hiding this comment.
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 🙂
|
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. |
Edit: I've since added a
.babelrc.jsfile 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.jsfile, 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.