Skip to content
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

feat!: migrate to ESM #397

Merged
merged 6 commits into from
Oct 23, 2023
Merged

feat!: migrate to ESM #397

merged 6 commits into from
Oct 23, 2023

Conversation

mdonnalley
Copy link
Contributor

BREAKING CHANGES: ESM and node 18 minimum

BREAKING CHANGES: ESM and node 18 minimum
@git2gus
Copy link

git2gus bot commented Oct 23, 2023

This issue has been linked to a new work item: W-14350069

return (hookFiles as string[]).map(h => `${path.resolve(h)}.ts`.replace(outDir, rootDir))
const hookFiles = Object.values(this.base.config.pjson.oclif?.hooks ?? {}).reduce(
(x: string[], y: string | string[]) => (Array.isArray(y) ? [...x, ...y] : [...x, y]),
[] as string[],
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
[] as string[],
[],

[] as string[],
)
const {outDir, rootDir} = this.getDirs()
return (hookFiles as string[]).map((h) => `${path.resolve(h)}.ts`.replace(outDir, rootDir))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return (hookFiles as string[]).map((h) => `${path.resolve(h)}.ts`.replace(outDir, rootDir))
return (hookFiles).map((h) => `${path.resolve(h)}.ts`.replace(outDir, rootDir))

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems like the types have been strengthened, but the usages are still using as ...


public async run(): Promise<CompareResponse> {
const {flags} = await this.parse(Compare)
const oldCommands = JSON.parse(fs.readFileSync(flags.filepath).toString('utf8')) as SnapshotEntry[]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know I'm guilty for writing some of these, but we can change to fs.readFileSync(flags..., 'utf-8') instead of using .toString - but that's a NIT

@WillieRuemmele
Copy link
Collaborator

QA Notes


snapshot

✅ : verified commands work in non-ESM plugin
✅ : compares snapshots correctly
✅ : generates new snapshot (new format)
✅ : compares new snapshot to old correctly

✅ : verified commands in ESM plugin
✅ : compares snapshots correctly
✅ : generates new snapshot (new format)
✅ : compares new snapshot to old correctly

schema

✅ : verified commands in ESM plugin
✅ : compares schema correctly
✅ : generates new schema (new format)
✅ : compares new schema to old correctly

✅ : verified commands in non-ESM plugin
✅ : compares schema correctly
✅ : generates new schema (new format)
✅ : compares new schema to old correctly

@WillieRuemmele WillieRuemmele merged commit b4cf7da into main Oct 23, 2023
6 checks passed
@WillieRuemmele WillieRuemmele deleted the mdonnalley/esm branch October 23, 2023 22:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants