bun test: "Cannot declare an imported binding name twice: 'Fragment'."

[gkiely]

What version of Bun is running?

1.0.16+800fb1290

What platform is your computer?

Darwin 21.6.0 arm64 arm

What steps can reproduce the bug?

Using a <></> and <Fragment> in the same file.

Sandbox: https://codesandbox.io/p/devbox/smoosh-wind-nwd8nq

What is the expected behavior?

Allows using both <></> and <Fragment /> syntax in the same file.

What do you see instead?

An error is reported

Additional information

Most of the time I use the shorthand fragment syntax <></> but a fragment within an array such as .map requires using a key, which the shorthand syntax does not support.

[krutoo]

same issue in bun run something.tsx

[sean256]

Same issue with bun test on v1.0.30

[Augustalex]

Same issue on latest version v1.0.33

[adubrouski]

Same issue on v1.0.35

I also get same error with createElement. In tsconfig.json I specify jsx: "react-jsx" . If I change it to just react - all will be work, but I need import it to large amount of files, in client bundle also.

[keverw]

I upgraded to v1.1.7 and ran into this just now :( I guess I had a old version where it worked, so went back to

curl -fsSl https://bun.sh/install | bash -s "bun-v1.0.30"

and now the same code works again, if that helps pinpoint where something might have changed.

I'm not using both <Fragment> and <> so maybe something I'm importing instead.

Edit: I decided to call it a night, and this afternoon I upgraded back to the newest version and start commenting things out. renderToString() on the server commented out and setting the html var to a blank string it built... Then I decided to just do a single <div></div> within renderToString and then it happened. I found another issue related to the minify, so decided to test that theory. I stopped minifying my server side bundle and now it works! So I guess my issue was more related to #9474 then. However frontend bundle seems ok to minify oddly... but not sure if it's even worth while to minify the server code, so just going to comment it out for now.

[shybovycha]

Same issue on 1.1.8

[shybovycha]

Apparently, this could be worked around, if configured properly in tsconfig.json by setting both

"jsx": "react",
"jsxFragmentFactory": "React.Fragment"

but then the failure becomes bun going to UMD modules instead of TS:

TS2686: React refers to a UMD global, but the current file is a module. Consider adding an import instead.

and

SyntaxError: export 'OptionGroupElement' not found in './OptionGroup'
      at /Users/shybovycha/projects/test1/frontend/node_modules/@leafygreen-ui/code/dist/index.js:1:4560

[shybovycha]

I have spent some time printf-debugging what goes into the parsed file AST and figured the JavaScriptParser does not perform de-duplication of the import statements in the parsed source code. By adding the following to the Cache.parse():

if (strings.eqlComptime(source.path.pretty, "/Users/shybovycha/projects/test1/frontend/node_modules/@leafygreen-ui/popover/src/Popover/Popover.tsx")) {
    for (result.ast.named_imports.values()) |*item| {
        debug(".ast.named_imports[] :: {s}", .{ item.*.alias.? });
    }
}

I was able to see that for one specific file there are two instances of Fragment in the result.ast.named_imports:

[fs] .ast.named_imports[] :: jsxDEV
[fs] .ast.named_imports[] :: Fragment // <<===========
[fs] .ast.named_imports[] :: forwardRef
[fs] .ast.named_imports[] :: Fragment // <<===========
[fs] .ast.named_imports[] :: useMemo
[fs] .ast.named_imports[] :: useState
[fs] .ast.named_imports[] :: Transition
[fs] .ast.named_imports[] :: css
[fs] .ast.named_imports[] :: cx
[fs] .ast.named_imports[] :: useIsomorphicLayoutEffect
[fs] .ast.named_imports[] :: useMutationObserver
[fs] .ast.named_imports[] :: useObjectDependency
[fs] .ast.named_imports[] :: usePrevious
[fs] .ast.named_imports[] :: useViewportSize
[fs] .ast.named_imports[] :: usePopoverContext
[fs] .ast.named_imports[] :: usePopoverPortalContainer
[fs] .ast.named_imports[] :: consoleOnce
[fs] .ast.named_imports[] :: createUniqueClassName
[fs] .ast.named_imports[] :: transitionDuration
[fs] .ast.named_imports[] :: Align
[fs] .ast.named_imports[] :: Justify
[fs] .ast.named_imports[] :: calculatePosition
[fs] .ast.named_imports[] :: getElementDocumentPosition
[fs] .ast.named_imports[] :: getElementViewportPosition

Moreover, looking into result.generated (which I assume might be causing the conflict with result.named_imports) it also seems to have an entry for Fragment. Can confirm that by changing the return statement of the JavaScriptParser._parse:

return js_ast.Result{ .ast = try p.toAST(parts_slice, exports_kind, wrapper_expr, hashbang) };

to

const ast = try p.toAST(parts_slice, exports_kind, wrapper_expr, hashbang);

if (strings.eqlComptime(p.source.path.pretty, "/Users/artem.shubovych/projects/migrator/frontend/node_modules/@leafygreen-ui/popover/src/Popover/Popover.tsx")) {
    for (0..ast.module_scope.generated.len) |idx| {
        const item_ref = ast.module_scope.generated.at(idx).*;
        const item = p.symbols.items[item_ref.innerIndex()];
        debug(".ast.module_scope.generated[] :: {s}", .{ item.original_name }); // >0
    }
}

return js_ast.Result{ .ast = ast };

to get

[JSParser] .ast.internal1.module_scope.generated[] :: Fragment / Fragment
[JSParser] .ast.internal1.module_scope.generated[] :: jsxDEV / jsxDEV
[JSParser] .ast.internal5.module_scope.generated[] :: jsx_dev_runtime / jsx_dev_runtime

in the logs.

To verify if the conflict between generated and named_imports might occur, I have modified the return part of the JavaScriptParser a bit more, to check if a statement from the generated list already exists in the named_imports:

debug(".ast.module_scope.generated[] :: {s}, named_imports.contains: {any}", .{ item.original_name, ast.named_imports.contains(item_ref) });

And indeed this is what I see in the logs:

[JSParser] .ast.module_scope.generated[] :: import_react, named_imports.contains: false
[JSParser] .ast.module_scope.generated[] :: import_react_transition_group, named_imports.contains: false
[JSParser] .ast.module_scope.generated[] :: import_prop_types, named_imports.contains: false
[JSParser] .ast.module_scope.generated[] :: emotion, named_imports.contains: false
[JSParser] .ast.module_scope.generated[] :: hooks, named_imports.contains: false
[JSParser] .ast.module_scope.generated[] :: leafygreen_provider, named_imports.contains: false
[JSParser] .ast.module_scope.generated[] :: lib, named_imports.contains: false
[JSParser] .ast.module_scope.generated[] :: portal, named_imports.contains: false
[JSParser] .ast.module_scope.generated[] :: tokens, named_imports.contains: false
[JSParser] .ast.module_scope.generated[] :: Popover, named_imports.contains: false
[JSParser] .ast.module_scope.generated[] :: positionUtils, named_imports.contains: false
[JSParser] .ast.module_scope.generated[] :: _JSXFrag$, named_imports.contains: false
[JSParser] .ast.module_scope.generated[] :: JSXFrag, named_imports.contains: false
[JSParser] .ast.module_scope.generated[] :: JSXFrag_3ccfcb2e132eae52, named_imports.contains: false
[JSParser] .ast.module_scope.generated[] :: _$jsx$, named_imports.contains: false
[JSParser] .ast.module_scope.generated[] :: $jsx, named_imports.contains: false
[JSParser] .ast.module_scope.generated[] :: $jsx_e0bdacae3797e8a2, named_imports.contains: false
[JSParser] .ast.module_scope.generated[] :: _jsxs$, named_imports.contains: false
[JSParser] .ast.module_scope.generated[] :: jsxs, named_imports.contains: false
[JSParser] .ast.module_scope.generated[] :: jsxs_32b5a867cce6d16e, named_imports.contains: false
[JSParser] .ast.module_scope.generated[] :: _jsxEl$, named_imports.contains: false
[JSParser] .ast.module_scope.generated[] :: jsxEl, named_imports.contains: false
[JSParser] .ast.module_scope.generated[] :: jsxEl_545b9f38f1fc3795, named_imports.contains: false
[JSParser] .ast.module_scope.generated[] :: _JSXClassic$, named_imports.contains: false
[JSParser] .ast.module_scope.generated[] :: JSXClassic, named_imports.contains: false
[JSParser] .ast.module_scope.generated[] :: JSXClassic_fe619c937e752e5e, named_imports.contains: false
[JSParser] .ast.module_scope.generated[] :: _JSX$, named_imports.contains: false
[JSParser] .ast.module_scope.generated[] :: JSX, named_imports.contains: false
[JSParser] .ast.module_scope.generated[] :: JSX_09d9fa208a136bc2, named_imports.contains: false
[JSParser] .ast.module_scope.generated[] :: Fragment, named_imports.contains: true // <<=======================
[JSParser] .ast.module_scope.generated[] :: jsxDEV, named_imports.contains: true // <<=======================
[JSParser] .ast.module_scope.generated[] :: jsx_dev_runtime, named_imports.contains: false

But not adding the import to the generated list if it is already present in the named_imports won't work - apparently, it happens earlier than named_imports gets filled out.

Then I looked at the named_imports once again, logging the entire entry struct itself:

[fs] .ast.named_imports[] :: Fragment :: src.js_ast.NamedImport{ .local_parts_with_uses = src.baby_list.BabyList(u32){ .ptr = u32@0, .len = 0, .cap = 0 }, .alias = { 70, 114, 97, 103, 109, 101, 110, 116 }, .alias_loc = src.logger.Loc{ .start = -1 }, .namespace_ref = Ref[0, 133, symbol], .import_record_index = 11, .alias_is_star = false, .is_exported = false }
[fs] .ast.named_imports[] :: Fragment :: src.js_ast.NamedImport{ .local_parts_with_uses = src.baby_list.BabyList(u32){ .ptr = u32@0, .len = 0, .cap = 0 }, .alias = { 70, 114, 97, 103, 109, 101, 110, 116 }, .alias_loc = src.logger.Loc{ .start = 28 }, .namespace_ref = Ref[0, 0, symbol], .import_record_index = 0, .alias_is_star = false, .is_exported = false }

There appear to be two entries with the same alias, but different locations in the hashmap. So comparing by the ref alone is not good enough.

Just for the experiment, I have added a short-circuit to the ImportScanner.scan() to not add an entry to the named_imports when it already contains an entry with the same alias:

const named_import_exists = find: {
    for (p.named_imports.values()) |*existing| {
        if (existing.*.alias) |existing_alias| {
            if (strings.eql(existing_alias, item.alias)) {
                break :find true;
            }
        }
    }

    break :find false;
};

if (!p.named_imports.contains(name_ref) and !named_import_exists) {
    continue;
}

And in the JavaScriptParser.toAST() I have added a code to clean up the module_scope.generated list:

if (p.module_scope.generated.len > 0) {
    var idx = p.module_scope.generated.len;

    while (idx > 0) {
        idx -= 1;

        const item_ref = p.module_scope.generated.at(idx).*;

        if (p.named_imports.contains(item_ref)) {
            p.module_scope.generated.remove(idx);
        }
    }
}

This last bit required modifying the BabyList struct to add the remove() function:

pub fn remove(this: *ListType, index: usize) void {
    bun.assert(index < this.len);
    bun.assert(this.len > 0);
    var list_ = this.listManaged(bun.default_allocator);
    _ = list_.orderedRemove(index);
    this.len -= 1;
}

After looking at the logs, seems like both lists do not contain more than one instance of Fragment import, but somehow the issue still persists.

Hopefully someone with a deeper knowledge of Bun can help out here.

[gabdara]

My workaround for now is to create a global.d.ts file with:

import { FunctionComponent } from 'preact';

declare global {
  const Fragment: FunctionComponent;
}

This way in the files I use Fragment I don't have to import it since it's already automatically imported by bun and TS doesn't complain it can't find the type.