Re-add "exports" declaration to package.json in backwards-compatible way #1028

cspotcode · 2020-05-04T04:23:59Z

coveralls · 2020-05-04T04:29:52Z

Coverage increased (+1.2%) to 83.019% when pulling f88fa6f on ab/fix-packagejson-exports-declaration into 3978f32 on master.

…exports-declaration

cspotcode · 2020-05-17T13:52:59Z

src/index.spec.ts

@@ -4,10 +4,12 @@ import { join } from 'path'
 import semver = require('semver')
 import ts = require('typescript')
 import proxyquire = require('proxyquire')
-import { register, create, VERSION } from './index'
+import * as tsNodeTypes from './index'


Type-only import, since we want to require() the locally-installed ts-node later.

import type possibly?

cspotcode · 2020-05-17T13:54:20Z

src/index.spec.ts

+    testsDirRequire.resolve('ts-node/register/transpile-only')
+    testsDirRequire.resolve('ts-node/register/type-check')
+    testsDirRequire.resolve('ts-node/tsconfig.schema.json')
+    testsDirRequire.resolve('ts-node/tsconfig.schemastore-schema.json')


Cannot load them, because they do things like install require hooks. But resolving should be enough to know that package.json "exports" is still compatible with legacy require() calls.

Should we remove some of these in the next major version?

Good point, I've already merged other breaking changes to master, so our next release will be major already.

Do you want me to tweak the "exports" object to only include a few?

Off the top of my head, here's a whitelist of the ones we should keep, what do you think @blakeembrey ?

ts-node // only reliably way to ask node for the root path of a dependency is Path.resolve(require.resolve('ts-node/package'), '..') ts-node/package // All bin entrypoints for people who need to augment our CLI: `node -r otherstuff ./node_modules/ts-node/dist/bin` ts-node/dist/bin ts-node/dist/bin-transpile ts-node/dist/bin-script // Must be require()able, obviously ts-node/register ts-node/register/files ts-node/register/transpile-only ts-node/register/type-check // Maybe this is useful? I dunno; I'd hate to break someone ts-node/tsconfig.schema

SGTM. I think we could safely omit tsconfig.schema but add it back if requested, but happy to keep it there for anyone using it as a manually referenced schema.

I'm assuming the reason we ship with tsconfig.schema at all is because of $ref or $schema in tsconfig.json? Maybe we could just omit it and have people reference a static version hosted outside the package going forward?

It's so you can add "$schema": "./node_modules/ts-node/tsconfig.schema.json" to your tsconfig file and know you are using the correct schema for the version of ts-node you are using. For example, if you're using an old or experimental build that changes which ts-node flags you are allowed to embed in a tsconfig, you'll want to refer to the bundled schema. Even then, you're probably referencing it via filename, not require().

For everyone else, their editor is using the schemastore schema by default, which already includes our stuff.

I'll omit it from "exports" and from the tests.

cspotcode · 2020-05-17T13:55:02Z

src/index.spec.ts

+        compilerOptions: {
+          jsx: 'preserve'
+        }
+      })


Since ts-node is now being loaded inside a before() callback, we must also register ts-node inside a before() callback.

cspotcode · 2020-05-17T13:55:25Z

src/index.spec.ts

@@ -637,10 +668,11 @@ describe('ts-node', function () {
    })

    describe('JSX preserve', () => {
-      let old = require.extensions['.tsx'] // tslint:disable-line
+      let old: (m: Module, filename: string) => any


Must be moved inside a before() callback.

…exports-declaration

codecov · 2020-05-20T00:37:06Z

Codecov Report

Merging #1028 into master will increase coverage by 1.15%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #1028      +/-   ##
==========================================
+ Coverage   73.68%   74.83%   +1.15%     
==========================================
  Files           6        6              
  Lines         608      608              
  Branches      142      142              
==========================================
+ Hits          448      455       +7     
+ Misses        101      100       -1     
+ Partials       59       53       -6

Flag	Coverage Δ
#node_10	`72.26% <ø> (+1.36%)`	⬆️
#node_12_15	`72.65% <ø> (+1.36%)`	⬆️
#node_12_16	`72.65% <ø> (+1.36%)`	⬆️
#node_13	`74.67% <ø> (+1.15%)`	⬆️
#node_14	`74.67% <ø> (+1.15%)`	⬆️
#typescript_2_7	`74.34% <ø> (+1.15%)`	⬆️
#typescript_latest	`73.51% <ø> (+1.15%)`	⬆️
#typescript_next	`73.35% <ø> (+1.15%)`	⬆️
#ubuntu	`74.50% <ø> (+1.15%)`	⬆️
#windows	`74.67% <ø> (+1.15%)`	⬆️

Impacted Files	Coverage Δ
src/index.ts	`77.08% <0.00%> (+2.08%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3978f32...f88fa6f. Read the comment docs.

blakeembrey · 2020-05-20T00:39:18Z

src/index.spec.ts

+    testsDirRequire.resolve('ts-node/register/transpile-only')
+    testsDirRequire.resolve('ts-node/register/type-check')
+    testsDirRequire.resolve('ts-node/tsconfig.schema.json')
+    testsDirRequire.resolve('ts-node/tsconfig.schemastore-schema.json')


Should we remove some of these in the next major version?

blakeembrey · 2020-05-22T03:19:42Z

src/index.spec.ts

@@ -4,10 +4,12 @@ import { join } from 'path'
 import semver = require('semver')
 import ts = require('typescript')
 import proxyquire = require('proxyquire')
-import { register, create, VERSION } from './index'
+import * as tsNodeTypes from './index'


import type possibly?

blakeembrey · 2020-05-22T03:20:55Z

package.json

+    "./dist/bin-transpile": "./dist/bin-transpile.js",
+    "./dist/bin-transpile.js": "./dist/bin-transpile.js",
+    "./dist/bin-script": "./dist/bin-script.js",
+    "./dist/bin-script.js": "./dist/bin-script.js",


Should we provide these as exports or only allow node node_modules/ts-node/dist/bin style? Do we want both .js and without?

I wasn't sure how people are using ts-node in the wild, and I couldn't think of any compelling benefits that outweighed the possibility of breaking something. Beyond that I don't have a strong opinion one way or the other.

Re-add "exports" declaration to package.json in backwards-compatible way

d58b94b

cspotcode added 7 commits May 17, 2020 07:27

Merge remote-tracking branch 'origin/master' into ab/fix-packagejson-…

af3e0d9

…exports-declaration

add package.json "exports" tests

ceb240f

more

56b8457

fix

3225918

fix

8057fb8

Merge remote-tracking branch 'origin/master' into ab/fix-packagejson-…

4548dcc

…exports-declaration

fix

daf12da

cspotcode commented May 17, 2020

View reviewed changes

cspotcode requested a review from blakeembrey May 17, 2020 13:56

cspotcode added 2 commits May 19, 2020 20:33

tweak test to check esm loader entrypoint

c0fc95d

Merge remote-tracking branch 'origin/master' into ab/fix-packagejson-…

311c008

…exports-declaration

blakeembrey approved these changes May 20, 2020

View reviewed changes

narrow list of exported files

3877ebc

cspotcode requested a review from blakeembrey May 20, 2020 02:48

blakeembrey approved these changes May 22, 2020

View reviewed changes

Update index.spec.ts

f88fa6f

cspotcode merged commit e567002 into master May 23, 2020

cspotcode mentioned this pull request May 24, 2020

ESM support: soliciting feedback #1007

Open

4 tasks

cspotcode deleted the ab/fix-packagejson-exports-declaration branch August 12, 2020 18:23

cspotcode mentioned this pull request Aug 21, 2020

release v9.0.0 #1118

Closed

4 tasks

This was referenced Mar 17, 2021

chore(deps-dev): bump ts-node from 8.10.2 to 9.1.1 in /showcase nicanac/stark#29

Closed

chore(deps-dev): bump ts-node from 8.10.2 to 9.1.1 in /deployments hollanddd/second-rate.website#3

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Re-add "exports" declaration to package.json in backwards-compatible way #1028

Re-add "exports" declaration to package.json in backwards-compatible way #1028

cspotcode commented May 4, 2020

coveralls commented May 4, 2020 •

edited

cspotcode May 17, 2020

blakeembrey May 22, 2020

cspotcode May 17, 2020

blakeembrey May 20, 2020

cspotcode May 20, 2020

blakeembrey May 20, 2020

blakeembrey May 20, 2020

cspotcode May 20, 2020 •

edited

cspotcode May 17, 2020

cspotcode May 17, 2020

codecov bot commented May 20, 2020 •

edited

blakeembrey May 20, 2020

blakeembrey May 22, 2020

blakeembrey May 22, 2020

cspotcode May 23, 2020 •

edited

Re-add "exports" declaration to package.json in backwards-compatible way #1028

Re-add "exports" declaration to package.json in backwards-compatible way #1028

Conversation

cspotcode commented May 4, 2020

coveralls commented May 4, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cspotcode May 20, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented May 20, 2020 • edited

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cspotcode May 23, 2020 • edited

Choose a reason for hiding this comment

coveralls commented May 4, 2020 •

edited

cspotcode May 20, 2020 •

edited

codecov bot commented May 20, 2020 •

edited

cspotcode May 23, 2020 •

edited