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

v2: "Got unexpected null" when building TypeScript project #3690

Closed
magcius opened this issue Oct 27, 2019 · 25 comments
Closed

v2: "Got unexpected null" when building TypeScript project #3690

magcius opened this issue Oct 27, 2019 · 25 comments

Comments

@magcius
Copy link

magcius commented Oct 27, 2019

🐛 bug report

When trying to upgrade my project to Parcel V2, I got the error message:

$ D:\Development\NITRO_BMD\node_modules\.bin\parcel build src/index.html
× Got unexpected null
    at nullthrows (D:\Development\NITRO_BMD\node_modules\nullthrows\nullthrows.js:7:15)
    at BundleGraph.resolveSymbol (D:\Development\NITRO_BMD\node_modules\@parcel\core\lib\BundleGraph.js:368:53)
    at BundleGraph.resolveSymbol (D:\Development\NITRO_BMD\node_modules\@parcel\core\lib\public\BundleGraph.js:116:51)
    at markUsed (D:\Development\NITRO_BMD\node_modules\@parcel\scope-hoisting\lib\concat.js:201:32)
    at enter (D:\Development\NITRO_BMD\node_modules\@parcel\core\lib\Graph.js:475:16)
    at enter (D:\Development\NITRO_BMD\node_modules\@parcel\core\lib\Graph.js:475:16)
    at enter (D:\Development\NITRO_BMD\node_modules\@parcel\core\lib\Graph.js:475:16)
    at walk (D:\Development\NITRO_BMD\node_modules\@parcel\core\lib\Graph.js:331:22)
⠹ Optimizing index.19203168.css...
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

🎛 Configuration (.babelrc, package.json, cli command)

The project is available here: https://github.com/magcius/noclip.website/tree/parcel-v2

🌍 Your Environment

Software Version(s)
Parcel 2.0.0-alpha.2.1
Node v10.16.3
npm/Yarn Yarn version 1.19.0
Operating System Windows 10
@mischnic
Copy link
Member

mischnic commented Oct 27, 2019

Very likely the same bug as #3640. (Also using @sentry/browser)

@magcius
Copy link
Author

magcius commented Oct 27, 2019

Thanks. I removed the sentry package for now, and updated the parcel-v2 branch with the removal. I am now hitting another error:

PS D:\Development\NITRO_BMD> yarn parcel build src\index.html
yarn run v1.19.0
$ D:\Development\NITRO_BMD\node_modules\.bin\parcel build src\index.html
× src\gfx\platform\GfxPlatformImpl.ts does not export 'GfxBindings'
    at replaceExportNode (D:\Development\NITRO_BMD\node_modules\@parcel\scope-hoisting\lib\link.js:149:13)
    at ReferencedIdentifier (D:\Development\NITRO_BMD\node_modules\@parcel\scope-hoisting\lib\link.js:511:20)
    at newFn (D:\Development\NITRO_BMD\node_modules\@babel\traverse\lib\visitors.js:216:17)
    at NodePath._call (D:\Development\NITRO_BMD\node_modules\@babel\traverse\lib\path\context.js:53:20)
    at NodePath.call (D:\Development\NITRO_BMD\node_modules\@babel\traverse\lib\path\context.js:40:17)
    at NodePath.visit (D:\Development\NITRO_BMD\node_modules\@babel\traverse\lib\path\context.js:88:12)
    at TraversalContext.visitQueue (D:\Development\NITRO_BMD\node_modules\@babel\traverse\lib\context.js:112:16)
    at TraversalContext.visitSingle (D:\Development\NITRO_BMD\node_modules\@babel\traverse\lib\context.js:84:19)
    at TraversalContext.visit (D:\Development\NITRO_BMD\node_modules\@babel\traverse\lib\context.js:140:19)
    at Function.traverse.node (D:\Development\NITRO_BMD\node_modules\@babel\traverse\lib\index.js:80:17)
⠧ Optimizing index.ee239b95.js...
error Command failed with exit code 1.

I don't know why it thinks that GfxBindings is not exported, when it clearly is.

Is there a way to turn scope hoisting off for now? All I could find was this, which implies it should be off by default: https://github.com/parcel-bundler/parcel/blob/v2/packages/core/parcel-bundler/src/Bundler.js#L110-L111

@mischnic
Copy link
Member

Is there a way to turn scope hoisting off for now? All I could find was this, which implies it should be off by default: /packages/core/parcel-bundler/src/Bundler.js@v2#L110-L111

--no-scope-hoist

(Even in the v2 branch, packages/core/parcel-bundler is the v1 version)

magcius added a commit to magcius/noclip.website that referenced this issue Oct 27, 2019
@magcius
Copy link
Author

magcius commented Oct 27, 2019

Thank you, I've disabled scope hoisting for now, which seems to have solved my problem. If you intend to look into the scope hoisting issues, please let me know if you need any help setting up the repo for a repro case.

@mischnic
Copy link
Member

mischnic commented Oct 27, 2019

× src\gfx\platform\GfxPlatformImpl.ts does not export 'GfxBindings'

Actually, the output from typescript for

export enum _T { Buffer, Texture, ColorAttachment, DepthStencilAttachment, Sampler, Program, Bindings, InputLayout, InputState, RenderPipeline };

interface GfxResourceBase { ResourceName?: string, ResourceUniqueId: number };
export interface GfxBuffer extends GfxResourceBase { _T: _T.Buffer };
export interface GfxTexture extends GfxResourceBase { _T: _T.Texture };
// ...
export type GfxResource =
    GfxBuffer | GfxTexture | GfxColorAttachment | GfxDepthStencilAttachment | GfxSampler | GfxProgram | GfxBindings | GfxInputLayout | GfxInputState | GfxRenderPipeline;

is

export var _T;
(function (_T) {
    _T[_T["Buffer"] = 0] = "Buffer";
    _T[_T["Texture"] = 1] = "Texture";
// ...
})(_T || (_T = {}));

Not even sure what the expected output is here (what is needed at runtime). But from Parcel's perspective, there is no GfxBindings ES6 named export.

I'm kind of surprised that works without scope hoisting. How does TS know that another modules exports aren't really named exports but an object property in a named export _T.

TS playground, and Babel doesn't support this at all? REPL

@magcius
Copy link
Author

magcius commented Oct 27, 2019

Interfaces are not a thing that have any runtime equivalent. They only exist in the typesystem.

@mischnic
Copy link
Member

I see, but Typescript can't know that

import { GfxDevice, GfxBufferUsage, GfxBuffer, GfxInputState, GfxFormat, GfxInputLayout, GfxProgram, GfxBindingLayoutDescriptor, GfxRenderPass, GfxBindings, GfxHostAccessPass, GfxVertexAttributeFrequency } from '../gfx/platform/GfxPlatform';

need to be completely removed with the typescript.transpileModule api (which processes every asset individually).

Flow has import type {...} for something like that. I don't know how Typescript solves this issue (related: #3624).

@magcius
Copy link
Author

magcius commented Oct 27, 2019

I don't know why babel REPL has issues with enums: https://babeljs.io/repl#?babili=false&browsers=IE%2011&build=&builtIns=false&spec=false&loose=false&code_lz=KYOwrgtgBAolDeUCGAaKAjNBjKBfA3AFBA&debug=false&forceAllTransforms=false&shippedProposals=false&circleciRepo=&evaluate=false&fileSize=false&timeTravel=false&sourceType=module&lineWrap=false&presets=typescript%2Cenv&prettier=false&targets=&version=7.6.4&externalPlugins=%40babel%2Fplugin-proposal-export-default-from%407.5.2

But offline, it can compile it just fine. I assume it just can't parse TypeScript online.

I don't know what exactly transpileModule does, but I assume it just ignores interfaces and doesn't emit them in the output, since it doesn't need to do any work to verify them. Note how it doesn't bother emitting the interfaces here: https://www.typescriptlang.org/play/index.html#code/JYWwDg9gTgLgBAbzgYQuCA7AphmAaOAJSwEMBjGAOQgBMsBBKKEgTzgF84AzKNOAciikK-ANwAocWQA2JAM5y4AMQgQ4WAB4wcNRanTZcicXFNwwAVwBG04GTgQrAKywU5ALiLCqtBk1YA2gC6cAC8cMES7OJAA

Why my code is different, I don't know.

@mischnic
Copy link
Member

I don't know what exactly transpileModule does, but I assume it just ignores interfaces and doesn't emit them in the output, since it doesn't need to do any work to verify them.

They aren't needed at runtime. But the issue here is that Typescript doesn't know if an import is importing a "real" value or an interface (where the export won't exist at runtime).

@magcius
Copy link
Author

magcius commented Oct 28, 2019

It will know, because an interface is never used except as a type annotation. You cannot declare functions on interfaces, nor construct them with new. They are not allowed in any grammar position that will appear in the transpiled JS, so it's implicit from the usage that the import can be dropped.

GfxBindings should never appear in any transpiled module's output. Why the scope hoisting code is seeing it is the real issue here.

@mischnic
Copy link
Member

mischnic commented Oct 28, 2019

Parcel could work around this Typescript problem by removing unused import specifiers (e.g. GfxBindings) while keeping the import declaration (because of side effects).

But generally, this (the Typescript output) is a violation of the ESM module spec (importing a non-existent export will throw a SyntaxError, it doesn't matter if it's unused).

@magcius
Copy link
Author

magcius commented Oct 28, 2019

Is the TypeScript output here generating an import for GfxBindings? That sounds like a TSC bug if it is.

@mischnic
Copy link
Member

Is the TypeScript output here generating an import for GfxBindings?

Yes (which is understandable because every file is transpiled individually and TS can't know if GfxBindings will exist at runtime. They'd have to remove an import if it's only used as a type annotation.)

@magcius
Copy link
Author

magcius commented Oct 28, 2019

I'll have to investigate later. If you can send me the TSC output or tell me where to find it, that would be a huge help. Not quite sure how to navigate the Parcel cache myself.

@magcius
Copy link
Author

magcius commented Oct 28, 2019

They'd have to remove an import if it's only used as a type annotation

TSC should be doing this already. See the example I posted above: https://www.typescriptlang.org/play/index.html#code/JYWwDg9gTgLgBAbzgYQuCA7AphmAaOAJSwEMBjGAOQgBMsBBKKEgTzgF84AzKNOAciikK-ANwAocWQA2JAM5y4AMQgQ4WAB4wcNRanTZcicXFNwwAVwBG04GTgQrAKywU5ALiLCqtBk1YA2gC6cAC8cMES7OJAA

@mischnic
Copy link
Member

Interesting, now the question is where the import isn't removed.

@devongovett
Copy link
Member

does babel do the same thing (remove type imports)? v2 uses babel to compile typescript by default, not tsc.

@mischnic
Copy link
Member

mischnic commented Oct 28, 2019

@devongovett his parcelrc:

{
    "extends": "@parcel/config-default",
    "transforms": {
        "*.ts": [ "@parcel/transformer-typescript-tsc", "@parcel/transformer-js" ]
    }
}

@magcius
Copy link
Author

magcius commented Oct 28, 2019

Yes, Babel's TypeScript support chokes on my project currently, so I cannot use it.

@Banou26
Copy link
Contributor

Banou26 commented Nov 15, 2019

Did anyone find a workaround for this error ?
I'm randomly getting it so i have to find ways to dev around it by not using certain features, which is quite annoying lol.

Banou26 added a commit to Banou26/EPK that referenced this issue Nov 17, 2019
@astegmaier
Copy link
Contributor

astegmaier commented Feb 28, 2020

I was able to solve this issue by removing two lines:

https://github.com/magcius/noclip.website/blob/c9f19e2206b035b957b63710577948bbb9ea2fb2/src/gfx/platform/GfxPlatform.ts#L324

https://github.com/magcius/noclip.website/blob/c9f19e2206b035b957b63710577948bbb9ea2fb2/src/viewer.ts#L247

(There are additional build errors that appeared after I solved this one, but they appear unrelated - I might be able to fully peel the onion if I have a bit more time).

The problem is that if you re-export types, you thwart typescript's ability to do "import elision" (i.e. detect whether an imported type is just used as a type (and can be removed by the transpiler) or not (and therefore should remain). This is a problem if you're compiling using transpileModules (like @parcel/transformer-typescript-tsc does). Typescript 3.8 added a new feature to allow you to explicitly say that you want to import a thing as a type, which can help work around this.

Assuming that parcel2 will be sticking with transpileModules (or babel, which has the same problem), it would be great if we could think of a way to give more actionable error messages when you hit this.

@magcius
Copy link
Author

magcius commented Feb 28, 2020

Ah, thank you, that makes sense. Re-exporting a type is a grammar position where types are allowed and can't be elided.

@astegmaier
Copy link
Contributor

@DeMoorJasper - I think you can close this because we found the root cause was by-design behavior. I opened #4240 to track the idea of providing more helpful error messages in this case.

@Richicratos
Copy link

I deleted the .parcel-cache folder and it works!

@LukasBombach
Copy link

You will also get this error in an isolated module, ie one that does neither import nor export anything

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants