Wrong exports in tslib

[jogibear9988]

I think the "import" specifier in the exports section should also point to ES6 Version:

tslib/package.json

Line 32 in 481d352

"import": "./modules/index.js",

see spec:
https://nodejs.org/api/packages.html#approach-1-use-an-es-module-wrapper

  Node.js implements the following conditions:
  
  "import" - matches when the package is loaded via import or import(), or via any top-level import or resolve operation by the ECMAScript module loader. Applies regardless of the module format of the target file. Always mutually exclusive with "require".
  "require" - matches when the package is loaded via require(). The referenced file should be loadable with require() although the condition matches regardless of the module format of the target file. Expected formats include CommonJS, JSON, and native addons but not ES modules as require() doesn't support them. Always mutually exclusive with "import".
  "node" - matches for any Node.js environment. Can be a CommonJS or ES module file. This condition should always come after "import" or "require".
  "node-addons" - similar to "node" and matches for any Node.js environment. This condition can be used to provide an entry point which uses native C++ addons as opposed to an entry point which is more universal and doesn't rely on native addons. This condition can be disabled via the --no-addons flag.
  "default" - the generic fallback that always matches. Can be a CommonJS or ES module file. This condition should always come last.

so there is no "module" specifier, but it is often used, I know. But if I want to import acording to spec, my site would load the one with "import" and then it would fail cause this is not usable by imports

[jogibear9988]

fix #162

[rbuckton]

tslib.es6.js predates NodeJS's support for ES Modules. If you change the "import" directive in the export map as you've proposed, NodeJS will attempt to load the file as a CommonJS module, resulting in the following error:

SyntaxError Unexpected token 'export'

The tslib.es6.js file can be used in the browser, but not in NodeJS.

We currently use "import": "./modules/index.js", as that file is in a directory with a package.json that includes { "type": "module" } so that NodeJS will treat it as an ES Module. That file simply re-exports the CommonJS tslib.js module.

From our tests, this is working as expected. What is the actual issue you are running into?

[jogibear9988]

My problem is, I don't use a package manager, I host directly the javascript emited from typescript, and this points to "./node_modules/tslib". Now my webserver follows the resolution strategy and rewrites the import to tslib.js, cause there is no definition of "module" for the export in the spec of nodejs

[jogibear9988]

Maybe switching the package.json to type"module" would fix this? so the export with "main" and "require" should point to a comonJS module then, and the rest to es6?

[rbuckton]

We can't change the root package.json in tslib to "type": "module" without breaking tslib.js. That's the reason we have a modules folder that has its own package.json.

[rbuckton]

Will #171 address your needs?

[jogibear9988]

I think this would work.

[jogibear9988]

thx

[justinfagnani]

Is there hope of getting this resolved even thought #171 has a disapproval?

As far as I know, "module" isn't one of the standard export conditions, and "import" is the condition that should point to a standard JS module.

We're hitting issues where we load tslib as a standard module, and get the error:

The requested module '../tslib' does not provide an export named 'default'

[thebanjomatic]

tslib.es6.js predates NodeJS's support for ES Modules. If you change the "import" directive in the export map as you've proposed, NodeJS will attempt to load the file as a CommonJS module, resulting in the following error:

SyntaxError Unexpected token 'export'

@rbuckton now that there is a tslib.es6.mjs file as well, could we just change things around so that file is used instead of modules/index.js

I'm currently running into issues after upgrading to vite 6 where my unit tests are failing because the following code is generated:

// node_modules/tslib/modules/index.js
var import_tslib = __toESM(require_tslib());
var {
  __extends,
  __assign,
  __rest,
  __decorate,
  __param,
  __esDecorate,
  __runInitializers,
  __propKey,
  __setFunctionName,
  __metadata,
  __awaiter,
  __generator,
  __exportStar,
  __createBinding,
  __values,
  __read,
  __spread,
  __spreadArrays,
  __spreadArray,
  __await,
  __asyncGenerator,
  __asyncDelegator,
  __asyncValues,
  __makeTemplateObject,
  __importStar,
  __importDefault,
  __classPrivateFieldGet,
  __classPrivateFieldSet,
  __classPrivateFieldIn,
  __addDisposableResource,
  __disposeResources,
  __rewriteRelativeImportExtension
} = import_tslib.default;

But default is not defined in this case and it should evidently be destructuring from import_tslib directly. I haven't 100% tracked things down, but it appears that in vite 5 the browser conditions were being applied, but in vite 6 the node conditions are so that at least explains the change in behavior (whether intentional or not).

The specifics of that issue aside, if I change the tslib export map to pull in tslib.es6.mjs directly, then the bundle only includes the helpers I'm using, and it doesn't need to go through the __toESM(require_tslib()) interop which at least unblocks me in this particular case.

Unless I'm missing something, I think you should be able to simplify the export map to the following:

      "exports": {
        ".": {
            "require": {
                "types": "./tslib.d.ts",
                "default": "./tslib.js"
            },
            "types": "./modules/index.d.ts",
            "default": "./tslib.es6.mjs"
        },
        "./*": "./*",
        "./": "./"
    }

That way import and require still get their own separate types and implementations, and I flipped things around to check explicitly for "require" so that you don't need to worry about non-standard things like "module" anymore and it can just be handled by the "default" case.

Would you be open to this change as a PR?

[Update]
Looking through, other issues and PRs, it seems loading the cjs code from node imports was a deliberate design decision, so I'm guessing not.