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

Help Wanted: Enable strictNullChecks with TypeScript #8852

Open
bigtimebuddy opened this issue Nov 15, 2022 · 4 comments
Open

Help Wanted: Enable strictNullChecks with TypeScript #8852

bigtimebuddy opened this issue Nov 15, 2022 · 4 comments
Labels
📢 Accepting PRs Would welcome a PR from the community.

Comments

@bigtimebuddy
Copy link
Member

bigtimebuddy commented Nov 15, 2022

Overview

We need some help if you're savvy in TypeScript! We would like to update our TypeScript implementation to enable strictNullChecks. Doing so would add a level of strictness to our code that everyone downstream would benefit from.

Below are the list of files affected.

Running

npm run types -- --strictNullChecks 
...

Found 1444 errors.

Filter By Package

First, add the path of the package to the pathPrefixs array in scripts/filterTypeScriptErrors.ts:

const pathPrefixs = [
'packages/utils',
];

Then run the following command:

npm run types:strict

You will see errors filtered by the specified packages.

If a PR want to enable strictNullChecks in a package, the path of the package should be added to the array in that PR. For further information, you can read the comments in scripts/filterTypeScriptErrors.ts, or PR #8965.

Files Affected

  • bundles/pixi.js-node/src/adapter/adapter.ts
  • bundles/pixi.js-node/src/adapter/loadNodeBitmapFont.ts
  • bundles/pixi.js-node/src/adapter/NodeCanvasElement.ts
  • bundles/pixi.js-node/src/adapter/NodeCanvasResource.ts
  • bundles/pixi.js-webworker/src/adapter.ts
  • packages/accessibility/src/AccessibilityManager.ts
  • packages/accessibility/src/accessibleTarget.ts
  • packages/accessibility/test/AccessibilityManager.tests.ts
  • packages/app/src/Application.ts
  • packages/app/src/ResizePlugin.ts
  • packages/app/test/Application.tests.ts
  • packages/assets/src/AssetExtension.ts
  • packages/assets/src/Assets.ts
  • packages/assets/src/BackgroundLoader.ts
  • packages/assets/src/cache/Cache.ts
  • packages/assets/src/loader/Loader.ts
  • packages/assets/src/loader/parsers/loadWebFont.ts
  • packages/assets/src/loader/parsers/textures/loadTextures.ts
  • packages/assets/src/loader/parsers/WorkerManager.ts
  • packages/assets/src/resolver/parsers/resolveTextureUrl.ts
  • packages/assets/src/resolver/Resolver.ts
  • packages/assets/test/loader.tests.ts
  • packages/basis/src/loader/BasisParser.ts
  • packages/basis/src/TranscoderWorker.ts
  • packages/basis/src/TranscoderWorkerWrapper.ts
  • packages/canvas-display/test/Container.tests.ts
  • packages/canvas-extract/src/CanvasExtract.ts
  • packages/canvas-graphics/src/CanvasGraphicsRenderer.ts
  • packages/canvas-graphics/test/CanvasGraphicsRenderer.tests.ts
  • packages/canvas-mesh/src/CanvasMeshRenderer.ts
  • packages/canvas-mesh/src/Mesh.ts
  • packages/canvas-mesh/src/NineSlicePlane.ts
  • packages/canvas-mesh/test/NineSlicePlane.tests.ts
  • packages/canvas-particle-container/src/ParticleContainer.ts
  • packages/canvas-prepare/src/CanvasPrepare.ts
  • packages/canvas-renderer/src/BaseTexture.ts
  • packages/canvas-renderer/src/CanvasContextSystem.ts
  • packages/canvas-renderer/src/CanvasObjectRendererSystem.ts
  • packages/canvas-renderer/src/CanvasRenderer.ts
  • packages/canvas-renderer/src/canvasUtils.ts
  • packages/canvas-renderer/src/utils/canUseNewCanvasBlendModes.ts
  • packages/canvas-sprite-tiling/src/TilingSprite.ts
  • packages/canvas-sprite/src/CanvasSpriteRenderer.ts
  • packages/canvas-sprite/src/Sprite.ts
  • packages/canvas-sprite/test/CanvasSpriteRenderer.tests.ts
  • packages/compressed-textures/src/loaders/loadKTX.ts
  • packages/compressed-textures/src/loaders/resolveCompressedTextureUrl.ts
  • packages/compressed-textures/src/parsers/parseKTX.ts
  • packages/compressed-textures/src/resources/BlobResource.ts
  • packages/compressed-textures/src/resources/CompressedTextureResource.ts
  • packages/core/src/batch/BatchDrawCall.ts
  • packages/core/src/batch/BatchGeometry.ts
  • packages/core/src/batch/BatchRenderer.ts
  • packages/core/src/batch/BatchSystem.ts
  • packages/core/src/batch/BatchTextureArray.ts
  • packages/core/src/batch/ObjectRenderer.ts
  • packages/core/src/context/ContextSystem.ts
  • packages/core/src/filters/Filter.ts
  • packages/core/src/filters/FilterState.ts
  • packages/core/src/filters/FilterSystem.ts
  • packages/core/src/filters/spriteMask/SpriteMaskFilter.ts
  • packages/core/src/framebuffer/Framebuffer.ts
  • packages/core/src/framebuffer/FramebufferSystem.ts
  • packages/core/src/framebuffer/GLFramebuffer.ts
  • packages/core/src/framebuffer/MultisampleSystem.ts
  • packages/core/src/geometry/Attribute.ts
  • packages/core/src/geometry/Buffer.ts
  • packages/core/src/geometry/BufferSystem.ts
  • packages/core/src/geometry/Geometry.ts
  • packages/core/src/geometry/GeometrySystem.ts
  • packages/core/src/geometry/GLBuffer.ts
  • packages/core/src/geometry/utils/interleaveTypedArrays.ts
  • packages/core/src/geometry/ViewableBuffer.ts
  • packages/core/src/mask/AbstractMaskSystem.ts
  • packages/core/src/mask/MaskData.ts
  • packages/core/src/mask/MaskSystem.ts
  • packages/core/src/mask/ScissorSystem.ts
  • packages/core/src/mask/StencilSystem.ts
  • packages/core/src/projection/ProjectionSystem.ts
  • packages/core/src/render/ObjectRendererSystem.ts
  • packages/core/src/Renderer.ts
  • packages/core/src/renderTexture/BaseRenderTexture.ts
  • packages/core/src/renderTexture/GenerateTextureSystem.ts
  • packages/core/src/renderTexture/RenderTexture.ts
  • packages/core/src/renderTexture/RenderTexturePool.ts
  • packages/core/src/renderTexture/RenderTextureSystem.ts
  • packages/core/src/shader/GLProgram.ts
  • packages/core/src/shader/Program.ts
  • packages/core/src/shader/Shader.ts
  • packages/core/src/shader/ShaderSystem.ts
  • packages/core/src/shader/UniformGroup.ts
  • packages/core/src/shader/utils/checkMaxIfStatementsInShader.ts
  • packages/core/src/shader/utils/compileShader.ts
  • packages/core/src/shader/utils/defaultValue.ts
  • packages/core/src/shader/utils/generateProgram.ts
  • packages/core/src/shader/utils/generateUniformBufferSync.ts
  • packages/core/src/shader/utils/getAttributeData.ts
  • packages/core/src/shader/utils/getMaxFragmentPrecision.ts
  • packages/core/src/shader/utils/getTestContext.ts
  • packages/core/src/shader/utils/getUniformData.ts
  • packages/core/src/shader/utils/logProgramError.ts
  • packages/core/src/shader/utils/mapType.ts
  • packages/core/src/state/State.ts
  • packages/core/src/state/StateSystem.ts
  • packages/core/src/system/SystemManager.ts
  • packages/core/src/textures/BaseTexture.ts
  • packages/core/src/textures/resources/AbstractMultiResource.ts
  • packages/core/src/textures/resources/ArrayResource.ts
  • packages/core/src/textures/resources/autoDetectResource.ts
  • packages/core/src/textures/resources/BaseImageResource.ts
  • packages/core/src/textures/resources/BufferResource.ts
  • packages/core/src/textures/resources/CubeResource.ts
  • packages/core/src/textures/resources/DepthResource.ts
  • packages/core/src/textures/resources/ImageBitmapResource.ts
  • packages/core/src/textures/resources/ImageResource.ts
  • packages/core/src/textures/resources/Resource.ts
  • packages/core/src/textures/resources/SVGResource.ts
  • packages/core/src/textures/resources/VideoResource.ts
  • packages/core/src/textures/Texture.ts
  • packages/core/src/textures/TextureGCSystem.ts
  • packages/core/src/textures/TextureSystem.ts
  • packages/core/src/transformFeedback/TransformFeedbackSystem.ts
  • packages/core/src/view/ViewSystem.ts
  • packages/core/test/BatchRenderer.tests.ts
  • packages/core/test/FilterSystem.tests.ts
  • packages/core/test/FramebufferSystem.tests.ts
  • packages/core/test/ImageResource.tests.ts
  • packages/core/test/MaskSystem.tests.ts
  • packages/core/test/ProjectionSystem.tests.ts
  • packages/core/test/Renderer.tests.ts
  • packages/core/test/RenderTexture.tests.ts
  • packages/core/test/RenderTextureSystem.tests.ts
  • packages/core/test/Shader.tests.ts
  • packages/core/test/ShaderSystem.tests.ts
  • packages/core/test/TextureSystem.tests.ts
  • packages/core/test/UniformBuffer.tests.ts
  • packages/display/src/Bounds.ts
  • packages/display/src/Container.ts
  • packages/display/src/DisplayObject.ts
  • packages/display/test/Container.tests.ts
  • packages/display/test/DisplayObject.tests.ts
  • packages/events/src/EventBoundary.ts
  • packages/events/src/EventSystem.ts
  • packages/events/src/FederatedEvent.ts
  • packages/events/src/FederatedEventTarget.ts
  • packages/events/src/FederatedMouseEvent.ts
  • packages/events/src/FederatedPointerEvent.ts
  • packages/events/src/FederatedWheelEvent.ts
  • packages/events/test/EventSystem.tests.ts
  • packages/extensions/src/index.ts
  • packages/extensions/test/extensions.tests.ts
  • packages/extract/src/Extract.ts
  • packages/extract/test/Extract.tests.ts
  • packages/filter-blur/src/BlurFilter.ts
  • packages/filter-blur/src/BlurFilterPass.ts
  • packages/filter-color-matrix/src/ColorMatrixFilter.ts
  • packages/graphics-extras/src/drawRoundedPolygon.ts
  • packages/graphics-extras/test/Graphics.tests.ts
  • packages/graphics/src/Graphics.ts
  • packages/graphics/src/GraphicsData.ts
  • packages/graphics/src/GraphicsGeometry.ts
  • packages/graphics/src/styles/FillStyle.ts
  • packages/graphics/src/utils/ArcUtils.ts
  • packages/graphics/src/utils/BatchPart.ts
  • packages/graphics/test/Graphics.tests.ts
  • packages/math-extras/src/index.ts
  • packages/math-extras/src/pointExtras.ts
  • packages/math-extras/src/rectangleExtras.ts
  • packages/math/test/Rectangle.tests.ts
  • packages/mesh-extras/src/SimpleMesh.ts
  • packages/mesh-extras/src/SimplePlane.ts
  • packages/mesh/src/Mesh.ts
  • packages/mesh/src/MeshBatchUvs.ts
  • packages/mesh/src/MeshMaterial.ts
  • packages/mixin-cache-as-bitmap/src/index.ts
  • packages/mixin-cache-as-bitmap/test/cacheAsBitmap.tests.ts
  • packages/mixin-get-child-by-name/src/index.ts
  • packages/particle-container/src/ParticleBuffer.ts
  • packages/particle-container/src/ParticleContainer.ts
  • packages/particle-container/src/ParticleRenderer.ts
  • packages/prepare/src/BasePrepare.ts
  • packages/runner/src/Runner.ts
  • packages/settings/src/settings.ts
  • packages/sprite-animated/src/AnimatedSprite.ts
  • packages/sprite-animated/test/AnimatedSprite.tests.ts
  • packages/sprite-tiling/src/TilingSprite.ts
  • packages/sprite-tiling/src/TilingSpriteRenderer.ts
  • packages/sprite-tiling/test/TilingSprite.tests.ts
  • packages/sprite/src/Sprite.ts
  • packages/spritesheet/src/Spritesheet.ts
  • packages/spritesheet/src/spritesheetAsset.ts
  • packages/spritesheet/test/Spritesheet.tests.ts
  • packages/spritesheet/test/spritesheetAsset.tests.ts
  • packages/text-bitmap/src/BitmapFont.ts
  • packages/text-bitmap/src/BitmapText.ts
  • packages/text-bitmap/src/formats/TextFormat.ts
  • packages/text-bitmap/src/formats/XMLFormat.ts
  • packages/text-bitmap/src/utils/extractCharCode.ts
  • packages/text-bitmap/test/BitmapFontLoader.tests.ts
  • packages/text-bitmap/test/BitmapText.tests.ts
  • packages/text/src/Text.ts
  • packages/text/src/TextMetrics.ts
  • packages/text/src/TextStyle.ts
  • packages/text/test/Text.tests.ts
  • packages/text/test/TextMetrics.tests.ts
  • packages/ticker/src/Ticker.ts
  • packages/ticker/src/TickerListener.ts
  • packages/ticker/src/TickerPlugin.ts
  • packages/ticker/test/TickerPlugin.tests.ts
  • packages/utils/src/browser/isWebGLSupported.ts
  • packages/utils/src/data/createIndicesForQuads.ts
  • packages/utils/src/media/CanvasRenderTarget.ts
  • packages/utils/src/media/trimCanvas.ts
  • packages/utils/src/network/decomposeDataUri.ts
  • packages/utils/src/network/getResolutionOfUrl.ts
  • packages/utils/src/path.ts
  • packages/utils/test/trimCanvas.tests.ts
  • test/index.ts
@bigtimebuddy bigtimebuddy added the 📢 Accepting PRs Would welcome a PR from the community. label Nov 15, 2022
@SuperSodaSea
Copy link
Member

I'd like to get involved.

@ASDFGerte
Copy link

Enabling strictNullChecks would be great. What's the plan here?

From what i know, removing the errors would work by simply adding | null | undefined to every type at the root of an error, and a non-null-assertion to the base of every member access, call, etc, where the inferrer cannot determine it isn't null or undefined.

While that would already help, the interesting part would be removing null or undefined from types, where these aren't desired. Also, non-null-assertions should be avoided - often the inferrer thinks the possibility of null or undefined still exists for a reason. That however is more complex, needs understanding of what every aspect of the code is doing, and may result in breaking changes (although somewhat benign ones).

Another thing is, that while rare, it may happen, that e.g. null is supposed to be assignable to something, but is never done in the pixi code-base. Then there wouldn't be any error, it would potentially be forgotten to add null to its type, and consumers also on strictNullChecks would get errors trying to assign null (although the API states it should be valid).

As mentioned, any step is imho a step in the right direction, and these can probably be done iteratively. I am curious though, what the intention is here. E.g. it would be within reach for me to do the first, but not the latter, as I still don't have a broad enough understanding of the pixi code-base.

@bigtimebuddy
Copy link
Member Author

Removing unnecessary null or undefined from types is definitely the objective (as well as being explicit about places where null is a valid input). This stuff will have to reviewed case-by-case. My suggestion, if someone is planning contributing to work on a single package at a time instead of some huge PR. It'll be easier to review and to make sure we aren't dramatically breaking.

We'll have to feel out the scope of the breaking changes as we go to decide what we're wiling to tolerate for a minor or patch release.

@8times12
Copy link

I'd like to contribute to this PR as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📢 Accepting PRs Would welcome a PR from the community.
Projects
None yet
Development

No branches or pull requests

4 participants