Uh oh!
There was an error while loading. Please reload this page.
Re-add "exports" declaration to package.json in backwards-compatible way - #1028
Conversation
| import ts = require('typescript') | ||
| import proxyquire = require('proxyquire') | ||
| import { register, create, VERSION } from './index' | ||
| import * as tsNodeTypes from './index' |
There was a problem hiding this comment.
Type-only import, since we want to require() the locally-installed ts-node later.
| 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') |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Should we remove some of these in the next major version?
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
| compilerOptions: { | ||
| jsx: 'preserve' | ||
| } | ||
| }) |
There was a problem hiding this comment.
Since ts-node is now being loaded inside a before() callback, we must also register ts-node inside a before() callback.
| describe('JSX preserve', () => { | ||
| let old = require.extensions['.tsx'] // tslint:disable-line | ||
| let old: (m: Module, filename: string) => any |
There was a problem hiding this comment.
Must be moved inside a before() callback.
Codecov Report
@@ 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
Continue to review full report at Codecov.
|
| 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') |
There was a problem hiding this comment.
Should we remove some of these in the next major version?
| import ts = require('typescript') | ||
| import proxyquire = require('proxyquire') | ||
| import { register, create, VERSION } from './index' | ||
| import * as tsNodeTypes from './index' |
| "./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", |
There was a problem hiding this comment.
Should we provide these as exports or only allow node node_modules/ts-node/dist/bin style? Do we want both .js and without?
There was a problem hiding this comment.
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.
See #1026