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!(generators): Fix generator typings #7727

Merged
merged 1 commit into from
Dec 19, 2023

Conversation

cpcallen
Copy link
Contributor

The basics

The details

Resolves

Fixes #7652.

Proposed Changes

  • Modify package_tasks.js to include tsc-generated .d.ts files for the generators in dist when building the blockly NPM package.
  • Replace the hand-written .d.ts files for the generator chunks with ones that reexport the types from the corresponding tsc-generated .d.ts file.

Reason for Changes

Publish accurate typings for the generators.

Additional Information

BREAKING CHANGE: Previously the dartGenerator, luaGenerator, javascriptGenerator, phpGenerator, and pythonGenerator exports (from the blockly/dart, blockly/lua, blockly/javascript, blockly/php and blockly/python entrypoints respectively) were being typed any. Now their types are correct. There is no change to their runtime behaviour, but developers who have written code in TypeScript that manipulates generators (adding custom block generator functions, for example) may find they need to correct type error in their code in order for it to compile correctly with Blockly v11.

Notably, block generator functions to be installed in the javascriptGenerator.forBlock dictionary should be typed

(block: Blockly.Block, generator: javascript.JavascriptGenerator) => [string, javascript.Order] | string | null

—and similarly for each of the other four languages we support.

@cpcallen cpcallen added component: generators breaking change Used to mark a PR or issue that changes our public APIs. PR: fix Fixes a bug labels Dec 18, 2023
@cpcallen cpcallen requested a review from a team as a code owner December 18, 2023 16:42
@cpcallen cpcallen requested review from NeilFraser and removed request for a team December 18, 2023 16:42
@github-actions github-actions bot added PR: fix Fixes a bug and removed PR: fix Fixes a bug breaking change Used to mark a PR or issue that changes our public APIs. labels Dec 18, 2023
@cpcallen cpcallen requested review from BeksOmega and removed request for NeilFraser December 19, 2023 18:24
@@ -348,7 +348,6 @@ function packageDTS() {
return gulp.src(handwrittenSrcs, {base: 'typings'})
.pipe(gulp.src(`${TYPINGS_BUILD_DIR}/**/*.d.ts`, {ignore: [
`${TYPINGS_BUILD_DIR}/blocks/**/*`,
`${TYPINGS_BUILD_DIR}/generators/**/*`,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Shouldn't this still exist if this is where the reexports are coming from?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is being deleted from the ignore: clause, thus causing it to be included in what gets copied from build/ to dist/.

Copy link
Collaborator

Choose a reason for hiding this comment

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

haha ok sweet sorry about that.

@cpcallen cpcallen merged commit 4dcf406 into google:rc/v11.0.0 Dec 19, 2023
11 checks passed
@cpcallen cpcallen deleted the fix/7652 branch December 19, 2023 21:21
@cpcallen
Copy link
Contributor Author

Darn it: this PR should have had a "!" in its title, and so especially should the squash commit, which didn't get a "BREAKING CHANGE:" text in the description. I'm going to rename the PR in the hopes that that is enough to make the release notes come out correctly. 🤞

@cpcallen cpcallen changed the title fix(generators): Fix generator typings fix!(generators): Fix generator typings Dec 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants