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

Runtime error when re-exporting empty module #6361

Closed
matt-tingen opened this issue May 26, 2021 · 12 comments · Fixed by #7049
Closed

Runtime error when re-exporting empty module #6361

matt-tingen opened this issue May 26, 2021 · 12 comments · Fixed by #7049

Comments

@matt-tingen
Copy link

matt-tingen commented May 26, 2021

🐛 bug report

When using parcel build, export * from './empty-module' in an esm dependency will result in a runtime error.

The error does not manifest when using parcel serve.

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

No parcel config, just parcel build src/index.html

🤔 Expected Behavior

No error

😯 Current Behavior

Runtime error such as Uncaught ReferenceError: $fd1dd7bd163658327eae64fa2f71c5d2$exports is not defined

💁 Possible Solution

Not sure, but if you think this would be an approachable first issue, I'd be happy to take a look.

🔦 Context

I ran into this with @chakra-ui/utils which has export * from './types' where types.ts is a file which only exports types so it compiles into an empty file. A repro using this chakra can be found in the repro repo.

As far as I can tell from the spec, empty modules are valid. I could be misunderstanding that, though.

💻 Code Sample

Repro repo

The additional foo import seems to be needed. Presumably this to prevent DCE.
The package boundary also appears to be necessary as far as I could tell.

🌍 Your Environment

Software Version(s)
Parcel 2.0.0-nightly.688
Node v14.15.0
npm/Yarn yarn 1.22.10
Operating System macos 10.15.7
@oeduardoal
Copy link

Hey @matt-tingen, I'm also going through this when exporting only type...

image

Link to repo: https://github.com/oeduardoal/error-parcel-empty-module

@oeduardoal
Copy link

But here, Its occurring in version "parcel": "2.0.0-beta.3.1"

@matt-tingen
Copy link
Author

Interestingly, my repro appears to be fixed in nightly 701+, as is import '@chakra-ui/utils'. However, import '@chakra-ui/styled-system' now yields a build-time error:

Check failed: result.second.
#
#
#
#FailureMessage Object: 0x70000ef6a8d0
 1: 0x10fefed72 node::NodePlatform::GetStackTracePrinter()::$_3::__invoke() [/Users/matttingen/.nodenv/versions/16.3.0/bin/node]
 2: 0x110edd1d2 V8_Fatal(char const*, ...) [/Users/matttingen/.nodenv/versions/16.3.0/bin/node]
 3: 0x1103285d4 v8::internal::GlobalBackingStoreRegistry::Register(std::__1::shared_ptr<v8::internal::BackingStore>) [/Users/matttingen/.nodenv/versions/16.3.0/bin/node]
 4: 0x11001fc76 v8::ArrayBuffer::GetBackingStore() [/Users/matttingen/.nodenv/versions/16.3.0/bin/node]
 5: 0x10fe69d8b node::Buffer::Data(v8::Local<v8::Value>) [/Users/matttingen/.nodenv/versions/16.3.0/bin/node]
 6: 0x1154723df EnvWrap::batchWrite(Nan::FunctionCallbackInfo<v8::Value> const&) [/Users/matttingen/Development/crossword/node_modules/lmdb-store/prebuilds/darwin-x64/node.abi93.node]
 7: 0x115473833 Nan::imp::FunctionCallbackWrapper(v8::FunctionCallbackInfo<v8::Value> const&) [/Users/matttingen/Development/crossword/node_modules/lmdb-store/prebuilds/darwin-x64/node.abi93.node]
 8: 0x110081805 v8::internal::FunctionCallbackArguments::Call(v8::internal::CallHandlerInfo) [/Users/matttingen/.nodenv/versions/16.3.0/bin/node]
 9: 0x110080dd8 v8::internal::MaybeHandle<v8::internal::Object> v8::internal::(anonymous namespace)::HandleApiCallHelper<false>(v8::internal::Isolate*, v8::internal::Handle<v8::internal::HeapObject>, v8::internal::Handle<v8::internal::HeapObject>, v8::internal::Handle<v8::internal::FunctionTemplateInfo>, v8::internal::Handle<v8::internal::Object>, v8::internal::BuiltinArguments) [/Users/matttingen/.nodenv/versions/16.3.0/bin/node]
10: 0x11008038f v8::internal::Builtin_Impl_HandleApiCall(v8::internal::BuiltinArguments, v8::internal::Isolate*) [/Users/matttingen/.nodenv/versions/16.3.0/bin/node]
11: 0x11090d299 Builtins_CEntry_Return1_DontSaveFPRegs_ArgvOnStack_BuiltinExit [/Users/matttingen/.nodenv/versions/16.3.0/bin/node]
error Command failed with signal "SIGILL".

This error does not occur when commenting out an export * from './types' so it's unclear to me how that differs from the cases resolved in 701.

The SIGILL can also be observed in oeduardoal's repro when updating parcel dependencies to the latest nightlies. As of this posting:

    "@parcel/packager-ts": "^2.0.0-nightly.716",
    "@parcel/transformer-typescript-types": "^2.0.0-nightly.716",
    "parcel": "^2.0.0-nightly.714",

@devongovett
Copy link
Member

I think the typescript types reexport issue should be fixed by #6414. Try a recent nightly and see if it works for you.

I'm not sure what the v8 issue you're seeing is. Are you sure it's related?

@mischnic
Copy link
Member

mischnic commented Jun 9, 2021

Check failed: result.second.

That will be fixed soon: kriszyp/lmdb-js#51

@matt-tingen
Copy link
Author

The hoisting issue does appear to be resolved. It seems that the empty modules can sometimes trigger the V8 error, but don't necessarily do so. However, since that error is tracked elsewhere, I'll go ahead and close this issue.

Thanks for all your work on parcel!

@matt-tingen
Copy link
Author

I'm reopening this because the issue still exists with empty modules, but has more specific preconditions now.

With the imdb-store issue fixed, I was able to test this on my full project. Unfortunately, I still ran into issue with Chakra, specifically the types re-export in @chakra-ui/utils.

I updated my repro to reflect the new preconditions as best I could figure out. It seems to require a peer re-export of the empty re-export to transitively re-export from another package.

@matt-tingen matt-tingen reopened this Jun 17, 2021
@devongovett devongovett added this to the v2.0.0-rc.0 milestone Jun 26, 2021
@devongovett
Copy link
Member

Unfortunately, I don't think there's much we can do here. If you add export {}; to empty.js it works. Parcel has to treat files without ESM exports as CommonJS, which means they export an object containing potentially unknown properties. The solution is to fix this in TypeScript compilers so that the output of a file containing only types still contains export {} which tells Parcel and other tools to treat the file as ESM. This is already the case in the official TSC compiler, and we recently contributed changes to SWC and Babel for this (see babel/babel#13314 in Babel v7.14.4). Now it's a matter of libraries upgrading their version and republishing their code. Probably a good idea to report this to Chakra UI.

@matt-tingen
Copy link
Author

Thanks for the update, Devon. Good to know there's a path forward; I've let chakra know.

For my own edification, do you mind clarifying why parcel has to treat such a file as CJS?
I was under the impression that specifying module marked the package as ESM and that that would apply to all constituent modules. Does importing or re-exporting a CJS package negate that in some way such that we can't determine what type of module empty.js is?

@devongovett
Copy link
Member

Right now, empty modules are treated the same as a module with the following:

module.exports = {};

This is required for compatibility with CommonJS. @mischnic might know if we could special case this in symbol propagation, but in general it gets complicated.

@devongovett
Copy link
Member

Minimal reproduction:

// index.js
import {mergeWith} from './other2';
console.log(mergeWith);

// other2.js
export * from './other';
export * from './empty';

// other.js
export { default as mergeWith } from "./mergeWith";

// mergeWith.js
module.exports = function mergeWith() {}

// empty.js

The combination of CJS module that assigns to module.exports (mergeWith.js) rather than being ESM is required, as is the empty module.

This results in the following used symbols:

  • mergeWith.js -> Set(1) { 'default' }
  • empty.js -> Set(1) { 'mergeWith' }

All other files have no used symbols.

This results in the following output:

(function () {
var $cd23135fd8be48a6$exports = {};
$cd23135fd8be48a6$exports = function mergeWith() {
};

var $3b0fc8b297effe95$exports = {};
console.log($ab990769c98d8a99$exports.mergeWith);
})();

This errors with ReferenceError: $ab990769c98d8a99$exports is not defined. $ab990769c98d8a99$exports is supposed to be the namespace object for other2.js, which seems correct to me: since we cannot resolve the symbol statically, we fall back to the namespace of the re-exporting asset. But it is not defined because there are no used symbols.

@mischnic do you think symbol propagation should somehow mark other2.js as having '*' in its used symbols? I think that would fix this problem and cause the namespace to be created in the packager, though it would essentially deopt tree shaking. I had a look at symbol propagation but tbh I'm not exactly sure where to do this. Any pointers?

@mischnic
Copy link
Member

Another smaller example:

// index.js
import { b } from "./other2";
console.log(b);

// other2.js
export * from "./other";
export * from "./empty";

// other.js
module.exports = { a: 1 };

// empty.js
module.exports = { b: 2 };

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

Successfully merging a pull request may close this issue.

4 participants