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

fix: manually add typings for generator classes #7824

Merged
merged 2 commits into from
Feb 2, 2024

Conversation

maribethb
Copy link
Contributor

The basics

The details

Resolves

Fixes missing generator types.

Proposed Changes

Exports type for the JavascriptGenerator class and other similar classes in the other generator languages.

Reason for Changes

Blockly exports both a JavascriptGenerator class, and a javascriptGenerator instance of that class. In most cases, folks use the instance. However, in some scenarios, a developer needs to access the class itself, whether to subclass or to use as a type in TypeScript. The class is exported and thus available in JavaScript, but it was not present in the .d.ts files and therefore invisible to TypeScript tooling.

The .d.ts files are currently manually maintained in Blockly v10. This is because for a while, generator classes were in JavaScript and no types were available. After converting them to TypeScript, more precise types are available, but it would be disruptive to TS developers to introduce them when previously the instance was typed as any, so we maintained the manual typing. In Blockly v11, we'll switch to generated type definitions that will include full information for both the class and instance of the class.

This PR only adds accurate typing for the class itself, not the instance. Because the class was not previously in TypeScript definitions at all, this isn't disruptive in the same way as it would be for the instance. If you wish to preview accurate typings for the instance as well as the class, please try the Blockly v11 beta by installing blockly@beta from npm.

Test Coverage

Added a TypeScript test file.

Documentation

None, other than release notes.

Additional Information

We'll plan to put this in a patch release for Blockly v10.

Note, if you arrive here due to a merge conflict between develop and the rc/v11 branch, you probably just want to accept the changes from rc/v11. This PR is a partial (non-breaking) implementation of the changes in #7727 and #7750

@maribethb maribethb requested a review from a team as a code owner February 1, 2024 21:55
@github-actions github-actions bot added the PR: fix Fixes a bug label Feb 1, 2024
Copy link
Contributor

@cpcallen cpcallen left a comment

Choose a reason for hiding this comment

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

I would have tried to copy and adapt the individual tests/typescript/src/generators/*.ts from #7727, but I see why they can't be used as-is (and perhaps you tried this already and found adaptation too difficult).

typings/dart.d.ts Show resolved Hide resolved
tests/typescript/src/generators.ts Show resolved Hide resolved
tests/typescript/src/generators.ts Show resolved Hide resolved
@maribethb maribethb merged commit d1cca3c into google:develop Feb 2, 2024
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: fix Fixes a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants