-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Make ESLint configuration aware of "env." #1827
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
edit: this was a false error likely due to the package linking used for testing
@peterp holy eslint refactor! Overall looking good to me. I ran some local tests:
- E2E = ✅
rw test
on the E2E project after run = TBDrw storybook
on the E2E project after run = ✅
**Previous `rw test` Error Message: Not Due to this PR**
rw test
mixed behavior
The command yarn rw test
fails with the following error:
$ rw test
yarn run v1.22.10
$ /private/var/folders/gj/_y54h7q11mz9rwnhn16h6yrr0000gn/T/redwood.o0cP1Pwp/node_modules/.bin/redwood test
/bin/sh: yarn: command not found
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
However, I can individually run either rw test api
or rw test web
. But then I run into strange test failures. E.g. for rw test web
I'm seeing things like:
FAIL web web/src/pages/AboutPage/AboutPage.test.js
● Test suite failed to run
Jest encountered an unexpected token
...
Details:
/Users/price/Repos/redwoodjs-redwood/packages/structure/dist/model/RWCell.js:3
import { withCell } from "@redwoodjs/web";
^^^^^^
SyntaxError: Cannot use import statement outside a module
33 | var _URL = require("../x/URL");
34 |
> 35 | var _RWCell = require("./RWCell");
| ^
36 |
37 | var _RWComponent = require("./RWComponent");
38 |
at Runtime.createScriptFromCode (../node_modules/jest-runtime/build/index.js:1350:14)
at Object.<anonymous> (../../../../../../../../Users/price/Repos/redwoodjs-redwood/packages/structure/dist/model/RWProject.js:35:15)
Something off between these changes and Structure package?
I'm seeing the Structure package referenced in all the test failure errors.
And the CLI Test command does use the Structure package to determine sides. See:
redwood/packages/cli/src/commands/test.js
Line 32 in 53bcdb2
.choices('side', getProject().sides) |
As for what could possibly be going on, I'll have to leave that answer in your hands 🤔
Suggestion: I think we'd improve the Test command by doing the same here. We could replace it with: .positional('side', {
choices: getProject().sides,
default: getProject().sides,
description: 'Which side(s) to test',
type: 'array',
}) |
@thedavidprice That seems good, but is this request related to this PR? |
… fix/dm-babel-register * 'fix/dm-babel-register' of github.com:dac09/redwood: Remove bundlesize dependency (redwoodjs#1844) upgrade execa; fix test.js cwd (redwoodjs#1846) Make ESLint configuration aware of "env." (redwoodjs#1827) Use createMany in Seed database example (redwoodjs#1776) Docs: Update contributor workflow to yarn rwt link (redwoodjs#1803) fix(page-loader): Adds types for pageLoadingContext | Fix for pageLoader during prerender (redwoodjs#1832) Minify CSS in production builds (redwoodjs#1697) New contributor workflow (redwoodjs#1792) Improve/template and auth setup tests (redwoodjs#1834)
Splits the ESLint configuration into two files:
In additional to spitting up the configuration we now set the environment that the code executes in, whereas before we set "node", "browser", and "jest" all to true, so globals like
window
would be available in every environment.This also disables usage of
window
in the RedwoodJS framework code.