-
Notifications
You must be signed in to change notification settings - Fork 197
Restore whole-program parsing/validation tests, with necessary def updates and repository maintenance
#886
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
Conversation
package.json
Outdated
| "glob": "8.0.3", | ||
| "mocha": "^9.1.3", | ||
| "recast": "0.21.5", | ||
| "recast": "0.23.0-pr-1247", |
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.
Intentional?
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.
Intentional but temporary: 3db28be
I am intentionally using the ^0.23.0 syntax here, even though other versions in devDependencies are exact (no ^), because recast has an exact dependency on ast-types. If there was an exact dependency in the other direction too, and the versions ever disagreed, multiple copies of these packages would need to be installed in nested node_modules directories, which is undesirable for many reasons.
These tests should be reporting an error according to output.json, but they both parse successfully. Possible clue: both tests have BABEL_8_BREAKING set to true in options.json.
| if (files.length < 10) { | ||
| throw new Error(`Unexpectedly few **/input.ts files matched (${ | ||
| files.length | ||
| }) in ${ | ||
| babelTSFixturesDir | ||
| }`); | ||
| } |
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.
Hopefully this check will prevent the scenario where the glob pattern suddenly fails to match any files (such as the upstream project deciding to bulk-rename all the input.ts files).
This minor version includes a major reorganization of the repository/package, which took place mostly in PR #327, with additional refinements in #886. There's always some risk to refactoring, and I added several new type definitions, hence the minor version bump. Why not a major version bump? I believe these changes should be mostly backwards compatible, especially if you are not importing from any nested directories like import es2020Plugin from "ast-types/def/es2020" Since the `def/` directory has moved to `lib/def/`, you may need to update your imports (a breaking change). However, you may be able to get away with no changes if you're using a version of Node.js (or a bundler) that understands the "exports" field in package.json: #327 (comment) Since I could be wrong about backwards compatibility, I will publish v0.16.0 to npm using the "next" dist-tag initially, so we can make sure benjamn/recast#1232 works. If breaking changes come to light, we should be able to fix them before triggering a flood of failing CI runs via dependabot/renovate/etc PRs.
This should address @eventualbuddha's comment #327 (comment), though of course reenabling those whole-program parsing and validations tests meant fixing the ones that were failing.
There are still two tests failing when checkingSkipped! e0ae66east.errorsagainstexpected.errors. If I can't figure out what's going on with those test fixture files tomorrow, I'll just skip parsing/validating them.New definitions added:
TSInstantiationExpressionTSTypeCastExpressionTSSatisfiesExpressionClassAccessorProperty