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

"exports" support backport #122

Closed
wants to merge 3 commits into from
Closed

Conversation

guybedford
Copy link
Contributor

This is a backport of #121 since the 1.0 version of tslib is still being used in many ES packages such as material etc.

//cc @weswigham @DanielRosenwasser

@dasa
Copy link

dasa commented Sep 4, 2020

ping 🙏🏻

@guybedford
Copy link
Contributor Author

Any chance of movement here @DanielRosenwasser @weswigham?

@orta
Copy link
Contributor

orta commented Sep 28, 2020

Hey folks, lets try get this moving. I've taken a look at this PR, but I can't get it to work in a way that I expected.

I haven't built an esmodules codebase before, so I cloned https://github.com/Urigo/typescript-node-es-modules-example as a reference using node esmodules and made some edits to make it use tslib for a function.

When I changed the tslib's package json to be the same as this lib running the code still failed with the same error:

$ node --experimental-specifier-resolution=node dist/index.js
file:///Users/ortatherox/dev/typescript/typescript-node-es-modules-example/dist/anotherFile.js:1
import { __awaiter } from "tslib";
         ^^^^^^^^^
SyntaxError: The requested module 'tslib' is expected to be of type CommonJS, which does not support named exports. CommonJS modules can be imported by importing the default export.
For example:
import pkg from 'tslib';
const { __awaiter } = pkg;
    at ModuleJob._instantiate (internal/modules/esm/module_job.js:98:21)
    at async ModuleJob.run (internal/modules/esm/module_job.js:137:5)
    at async Loader.import (internal/modules/esm/loader.js:162:24)
error Command failed with exit code 1.

But when I switched the tslib setup to replicate #119 it ran fine:

❯ yarn start
yarn run v1.22.4
$ node --experimental-specifier-resolution=node dist/index.js
hello world
Inside new string from function in another file

Considering you are a domain expert, and I'm an hour or two into researching. Am I missing something important, or is there a use case I've overlooked?

@DanielRosenwasser
Copy link
Member

DanielRosenwasser commented Sep 29, 2020

Can you please undo the rename to .cjs?

@orta
Copy link
Contributor

orta commented Sep 29, 2020

Yeah, bit of a rock & hard place here - "module": true will mean JS files are a module by default ( e.g. .js files) and in that context .cjs is commonjs.

@guybedford checked on npm and no-one is using tslib/tslib.js but I think there's probably just too many private codebase who are probably doing weird things which we don't want to break unintentionally

I think the only answer is duplication of either tslib.es6.js -> tslib.mjs or tslib.js -> tslib.cjs with module: true TBH

@ljharb
Copy link

ljharb commented Sep 29, 2020

Why is type: module needed at all? It's totally fine to use the default behavior, that .mjs is ESM and .js is CJS.

@orta
Copy link
Contributor

orta commented Sep 29, 2020

That was to avoid a duplicate file - @ljharb see #126 for an evolution of this PR

@orta
Copy link
Contributor

orta commented Oct 6, 2020

Thanks folks, the new backport PR is here #127 - I'm going to close this

@orta orta closed this Oct 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants