mirror of
https://github.com/ezolenko/rollup-plugin-typescript2.git
synced 2025-12-08 19:06:16 +00:00
31 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
Anton Gilgur
|
a1ae42b949
|
test: increase no-errors integration timeout to 20s (#425)
- this was still occassionally failing on CI at 15s, so bump up a bit - c.f. https://github.com/ezolenko/rollup-plugin-typescript2/actions/runs/3075482590/jobs/4968935300, https://github.com/ezolenko/rollup-plugin-typescript2/actions/runs/3038898669/jobs/4893183507, https://github.com/ezolenko/rollup-plugin-typescript2/actions/runs/3038898669/jobs/4893561438, and many more - only `no-errors` was timing out, which makes sense, since it does 3 builds for the first test to test against the cache - this potentially could be optimized, but it started getting complicated to do so (including some concurrency issues) - so punting on that for now and just increasing the timeout |
||
|
Anton Gilgur
|
e8f59ef485
|
fix: don't error out while catching a buildStart error (#422)
- per in-line comment, if an error occurs during `buildStart` initialization, then the `cache` may not exist yet during `buildDone`
- since we now use `context.error` instead of `throw` during initialization (from the `options` -> `buildStart` change), `buildEnd` will run during initialization
- and the `cache` var is initialized in `buildStart` as well, so if an error occurs before then, the `cache` won't exist
- we should gracefully handle this in all cases, since it's possible that even creating the `cache` could throw an error
- this error was hiding the underlying error, which was problematic for DX as well as bug reports (see my issue follow-up)
- add an integration test for `tsconfig` to make sure this code works
- while this is similar to the previous `tsconfig` integration tests (that were moved to unit tests), this covers all cases of `buildStart` errors, and is not specific to the different `tsconfig` errors (unlike the unit tests)
- this test will fail without the source code changes in this commit
|
||
|
Anton Gilgur
|
ba262937f8
|
refactor: consolidate diagnostics funcs into single file (#416)
* refactor: consolidate `diagnostics` funcs into single file
- move `IDiagnostic` and `convertDiagnostic` from `tscache` to `print-diagnostics`, then rename it to `diagnostics.ts`
- the diagnostic funcs being in `tscache` always felt like a strange place for them
- especially when `parse-tsconfig` or `print-diagnostics` would import them from `tscache`, which sounded like an unrelated file
- may want to move `diagnostics-format-host` into this file as well, as it is _only_ used in this file
- leaving as is for now, limiting this change to a smaller one
* test: add unit test for `convertDiagnostic`
- this was previously only covered in integration tests
- since unit tests don't currently touch `index` or `tscache`, and `convertDiagnostic` was previously in `tscache`
- another reason why consolidating these functions into one file made sense
* fix(diagnostics): use `formatHost.getNewLine()` instead of `\n`
- since new lines are host OS specific
- this fixes the `convertDiagnostic` test on Windows too, where it was failing
|
||
|
Anton Gilgur
|
560ed8dac7
|
fix: handle all type-only imports by piping TS imports (#406)
* fix: handle all type-only imports by piping TS imports
- `result.references` is populated by `ts.preProcessFile`; i.e. this is TS discovering all imports, instead of Rollup
- TS's imports include type-only files as TS understands those (whereas they aren't emitted in the JS for Rollup to see, since, well, they produce no JS)
- so we can pipe all these through Rollup's `this.resolve` and `this.load` to make them go through Rollup's `resolveId` -> `load` -> `transform` hooks
- this makes sure that other plugins on the chain get to resolve/transform them as well
- and it makes sure that we run the same code that we run on all other files on type-only ones too
- for instance: adding declarations, type-checking, setting them as deps in the cache graph, etc
- yay recursion!
- also add check for circular references b/c of this recursion (which Rollup docs confirm is necessary, per in-line comment)
- and Rollup ensures that there is no perf penalty if a regular file is processed this way either, as it won't save the hook results when it appears in JS (i.e. Rollup's module graph)
- we are checking more files though, so that in and of itself means potential slowdown for better correctness
- add a test for this that uses a `tsconfig` `files` array, ensuring that the `include` workaround won't cover these type-only files
- this test fails without the new code added to `index` in this commit
- also add another file, `type-only-import-import`, to the `no-errors` fixture to ensure that we're not just checking imports one level deep, and actually going through type-only imports of type-only imports as well
- the declaration check for this will fail if type-only imports are not handled recursively
- an initial version of this fix that I had that didn't call `this.load` failed this check
- refactor(test): make the integration tests more resilient to output ordering changes
- due to the eager calls to `this.load`, the ordering of declaration and declaration map outputs in the bundle changed
- and bc TS's default ordering of imports seems to differ from Rollup's
- note that this only changed the order of the "bundle output" object -- which Rollup doesn't guarantee ordering of anyway
- all files are still in the bundle output and are still written to disk
- for example, the `watch` tests did not rely on this ordering and as such did not need to change due to the ordering change
- create a `findName` helper that will search the `output` array instead, ensuring that most ordering does not matter
- we do still rely on `output[0]` being the bundled JS (ESM) file, however
- refactor(test): go through a `files` array for tests that check for multiple files instead of listing out each individual check
- this makes the tests more resilient to fixture changes as well (i.e. addition / deletion of files)
- create `no-errors.ts` that exports a list of files for this fixture
- didn't need to do the same for `errors.ts` as of yet; may do so in the future though
* move type-only recursion to after declarations are added to the `declarations` dict
- preserve some ordering and simplify future debugging
- also fix lint issue, `let modules` -> `const modules`
- I previously changed it (while WIP), but now it's static/never reassigned, so can use `const`
* rewrite the core logic with a `for...of` loop instead
- simpler to follow than `map` + `filter` + `Promise.all`
- might(?) be faster without `Promise.all` as well as more can happen async without waiting
- (I'm not totally sure of the low-level implementation of async to know for sure though)
* add comment about normalization
|
||
|
Anton Gilgur
|
1e71d5064d
|
clean: remove ConsoleContext entirely by using buildStart (#414)
- the only reason that `ConsoleContext` was still used was because the `options` hook doesn't support certain context functions like `this.error` / `this.warn` etc
- but `buildStart` does! so we can just move pretty much everything into `buildStart` instead
- didn't move setting `rollupOptions` because that value gets hashed and the value in `buildStart` is different, as it is after all plugins' `options` hooks have ran
- this way we don't need `ConsoleContext` what-so-ever and so can remove it entirely!
- as well as `IContext`, its tests, etc
- use `this.error` instead of `throw`ing errors in `parse-tsconfig` as it exists now
- couldn't in the `options` hook, can in `buildStart`
- this also ensure we don't have all the `rpt2: ` prefixes ever missing again
- c.f. ff8895103c8466694e7d8eeb734f51d2850670d8, 0628482ea26ddf56e6ef5521e4dcb85ef5b4beb6
- refactor `parse-tsconfig.spec` to account for this change
- c.f. Rollup hooks docs: https://rollupjs.org/guide/en/#options, https://rollupjs.org/guide/en/#buildstart
|
||
|
Anton Gilgur
|
79053fe82f
|
refactor: combine check-tsconfig with parse-tsconfig (#413)
- `checkTsConfig` is literally only used by `parseTsConfig` - I first just moved the function over, but it's a two-liner, so thought it made more sense to just in-line it - merge the unit tests as well - we already check the non-error case in `parse-tsconfig.spec`, so only add the error case - `check-tsconfig` was longer in the past and more files were being created then for separation, so this may have made more sense then, but now it is unnecessary - c.f. 733207791800ffe15672554d2a6337c73c65cf74 |
||
|
Anton Gilgur
|
27356be3a0
|
test: add print-diagnostics spec (#405)
- test all the different categories and formattings - full unit test coverage now! (except for `index`/`tscache`, which integration tests cover) |
||
|
Anton Gilgur
|
0c8e88d42c
|
refactor(test): heavily simplify the context helper (#404)
* refactor(test): heavily simplify the `context` helper
- since we're type-casting it anyway, we can heavily simplify this and remove the stubs entirely
- they're actually unused in the unit tests, so we don't need them at all
- besides the type-checking, which we force with a cast anyway
- the `as unknown as` is bad practice, and probably why I didn't use it initially (plus other typing issues), but it's much simpler this way and reflects the intent better -- just making it type-check with the few properties we use
- we can also use Jest mocks directly instead of the hacky `contextualLogger` and passing `data` in
- `makeContext` now creates the mocks, so we just need to check against `context.error` etc
- this is _much_ more familiar as it's what we use in the source and follows idiomatic Jest
- rewrite all the checks to test against the mocks instead
- I thought this felt too complicated / verbose before, but I had left this as is from brekk's initial test structure
- now that I understand all the tests and test intent much better, I could rewrite this to be a good bit simpler
- make the `toBeFalsy()` checks more precise by checking that the mock _wasn't_ called
- it returns `void` anyway, so `toBeFalsy()` _always_ returns true; it's not much of a test
- checking that the low verbosity level didn't trigger the mock to be called actually checks the test's intent
* make sure to check funcs in context calls too
- `get-options-overrides`'s coverage func % decreased bc funcs passed to `context.debug` weren't being called
- took me a bit to notice too since we have no coverage checks
- and then another bit to realize _why_ it decreased
|
||
|
Anton Gilgur
|
83835009f4
|
test: ensure declarationMap sources are correct (#403)
- since we set a placeholder `outDir` initially, TS sets declaration map sources to that placeholder dir
- then, after ec0568ba2c8e206372f94164e697b1469bf3f33d, we remap that to the correct dir that Rollup outputs to
- this test checks that the remap happens and is correct
|
||
|
Anton Gilgur
|
4e5ab742ea
|
test: 100% coverage for tslib.ts (error case) (#399)
- use Jest's module mocks to test the error case of `tslib.ts` when `tslib` fails to import
- this took a bit of work to figure out bc, per the in-line comments, the ordering and module subpath were both _required_
- it was pretty confusing for a while until I realized what might be going wrong
|
||
|
Anton Gilgur
|
576558e082
|
refactor(test): use more specific checks in check-tsconfig spec (#398)
- `toBeFalsy()` was a bit imprecise and always felt as such, especially since `checkTsConfig` returns `void` - so instead check that it doesn't throw, which matches the test _intent_ - reorder the tests a bit to match existing test style: non-errors first, then errors - and separating into two different test blocks parallelizes them as well - also add an ES2020 check as well (follow-up to eb1dd17babde0b22e9540b12e671eb56d9d6bce0) |
||
|
Anton Gilgur
|
d286015ad2
|
test: add parse-tsconfig spec (#397)
* test: add `parse-tsconfig` spec - test passing tsconfig, buggy tsconfig, non-existent tsconfig, and not a tsconfig - clean: "failed to read" error will never happen as we already checked for existence of the file earlier - so remove the `undefined` check and instead use a non-null assertion (plus a comment explaining it) - refactor: move the integration test for tsconfig error into this unit test instead - faster / more efficient / more precise - refactor: split out a `makeOptions` func that creates default plugin options to use in tests - similar to `makeStubbedContext` * fix windows test by normalizing Co-authored-by: Eugene Zolenko <zolenkoe@gmail.com> |
||
|
Anton Gilgur
|
c1f3a35dea
|
refactor: combine two context files into one (#396)
* refactor: combine two context files into one - they're virtually identical, so combine them instead of keeping them separate - changes to one should probably be made to both - still a < 100 LoC file - refactor out `_.isFunction` with a simple `getText` function instead - checks the opposite - one more lodash removal! - add docstrings about when to use the two contexts * disable tslint rule for this file |
||
|
Anton Gilgur
|
fd6f195834
|
test: add initial watch mode test suite (#386)
- test starting watch mode, changing a file, and adding a semantic error
- put this in a separate file as it has its own complexity to deal with (see below)
- initially put it inside `no-errors`, but it got pretty confusing; this structure is a good bit simpler
- refactored this a couple times actually
- add two watch mode helpers
- `watchBundle` is very similar to `genBundle` but with some watch mode nuances (like returning a watcher)
- `watchEnd` is a bit complicated async wrapper around a listener interface to make imperative testing simpler
- refactor: abstract out `createInput` and `createOutput` to be used in both `genBundle` and `watchBundle`
- refactor: make sure `dist` output gets put into a temp test dir
- the previous config made it output into rpt2's `dist` dir, since `cwd` is project root (when running tests from project root)
- the Rollup API's `watch` func always writes out; can't just test in-memory like with `bundle.generate`
- so the `dist` dir becomes relevant as such
- refactor: pass in a temp `testDir` instead of the `cacheRoot`
- we can derive the `cacheRoot` and the `dist` output from `testDir`
- also improve test clean-up by removing `testDir` at the end, not just the `cacheRoot` dir inside it
- `testDir` is the same var used in the unit tests to define a temp dir for testing
- change the `no-errors` fixture a tiny bit so that changing the import causes it to change too
- this ended up being fairly complex due to having to handle lots of async and parallelism
- parallel testing means fixtures have to be immutable, but watch mode needs to modify files
- ended up copying fixtures to a temp dir where I could modify them
- async events are all over here
- had to convert a callback listener interface to async too, which was pretty confusing
- and make sure the listener and bundles got properly closed too so no leaky tests
- apparently `expect.rejects.toThrow` can return a Promise too, so missed this error for a while
- refactor: make sure the error spec awaits too (though I think the errors _happen_ to throw synchronously there)
- and also found a number of bugs while attempting to test cache invalidation within watch mode
- thought it was a natural place to test since watch mode testing needs to modify files anyway
- had to trace a bunch to figure out why some code paths weren't being covered; will discuss this separately from this commit
- testing Rollup watch within Jest watch also causes Jest to re-run a few times
- I imagine double, overlapping watchers are confusing each other
- might need to disable these tests when using Jest watch
- high complexity async + parallel flows makes it pretty confusing to debug, so this code needs to be "handled with care"
- also this high complexity was even harder to debug when I'm having migraines (which is most of the time, unfortunately)
- hence why it took me a bit to finally make a PR for this small amount of code; the code was ok, the debugging was not too fun
|
||
|
Anton Gilgur
|
c2b3db2300
|
fix: don't duplicate type-only check on Windows (#385)
- gotta remember to normalize file names 🙃
- this fixes a failing test on Windows by fixing the underlying bug
- refactor: improve normalization in errors.spec
- normalize _all_ calls to `local`, instead of just one
- this doesn't affect the tests, only the source code change does, but I added this first and it's better practice / test future-proofing
|
||
|
Anton Gilgur
|
eab48a3951
|
fix: type-check included files missed by transform (type-only files) (#345)
* fix: type-check `include`d files missed by transform (type-only files)
- type-only files never get processed by Rollup as their imports are
elided/removed by TS in the resulting compiled JS file
- so, as a workaround, make sure that all files in the `tsconfig`
`include` (i.e. in `parsedConfig.fileNames`) are also type-checked
- note that this will not catch _all_ type-only imports, in
particular if one is using `tsconfig` `files` (or in general _not_
using glob patterns in `include`) -- this is just a workaround,
that requires a separate fix
- we do the same process for generating declarations for "missed"
files right now in `_onwrite`, so basically do the same thing but
for type-checking in `_ongenerate`
(_technically_ speaking, there could be full TS files
(i.e. _not_ type-only) that are in the `include` and weren't
transformed
- these would basically be independent TS files not part of the bundle
that the user wanted type-checking and declarations for (as we
_already_ generate declarations for those files))
* move misssed type-checking to `buildEnd` hook, remove declarations check
- `buildEnd` is a more correct place for it, since this does not generate any output files
- (unlike the missed declarations)
- and this means it's only called once per build, vs. once per output
- remove the check against the `declarations` dict as one can type-check without outputting declarations
- i.e. if `declaration: false`; not the most common use-case for rpt2, but it is one
* add new checkedFiles Set to not duplicate type-checking
- since `parsedConfig.fileNames` could include files that were already checked during the `transform` hook
- and because `declarations` dict is empty when `declaration: false`, so can't check against that
* move checkedFiles.add to the beginning of typecheckFile
- because printing diagnostics can bail if the category was error
- that can result in a file being type-checked but not added to checkedFiles
* wip: fuse _ongenerate functionality into buildEnd, _onwrite into generateBundle
- per ezolenko, the whole generateRound etc stuff was a workaround because the buildEnd hook actually _didn't exist_ before
- so now that it does, we can use it to simplify some logic
- no longer need `_ongenerate` as that should be in `buildEnd`, and no longer need `_onwrite` as it is the only thing called in `generateBundle`, so just combine them
- importantly, `buildEnd` also occurs before output generation, so this ensures that type-checking still occurs even if `bundle.generate()` is not called
- also move the `walkTree` call to above the "missed" type-checking as it needs to come first
- it does unconditional type-checking once per watch cycle, whereas "missed" only type-checks those that were, well, "missed"
- so in order to not have duplication, make "missed" come after, when the `checkedFiles` Set has been filled by `walkTree` already
- and for simplification, just return early on error to match the current behavior
- in the future, may want to print the error and continue type-checking other files
- so that all type-check errors are reported, not just the first one
NOTE: this is WIP because the `cache.done()` call and the `!noErrors` message are incorrectly blocked behind the `pluginOptions.check` conditional right now
- `cache.done()` needs to be called regardless of check or error or not, i.e. in all scenarios
- but ideally it should be called after all the type-checking here
- `!noErrors` should be logged regardless of check or not
- and similarly, after the type-checking
* call `cache().done()` and `!noErrors` in check and non-check conditions
- instead of making a big `if` statement, decided to split out a `buildDone` function
- to always call at the end of the input phase
- we can also move the `cache().done()` in `emitSkipped` into `buildEnd`, as `buildEnd` gets called when an error occurs as well
- and this way we properly print for errors as well
- `buildDone` will have more usage in other PRs as well, so I figure it makes sense to split it out now as well
* use `RollupContext` for type-only files
- i.e. bail out when `abortOnError: true`, which `ConsoleContext` can't do
- `ConsoleContext` is basically meant for everywhere `RollupContext` can't be used
- which is effectively only in the `options` hook, per the Rollup docs: https://rollupjs.org/guide/en/#options
* add test for type-only file with type errors
- now that the integration tests exist, we can actually test this scenario
- refactor: give each test their own `onwarn` mock when necessary
- while `restoreMocks` is set in the `jest.config.js`, Jest apparently has poor isolation of mocks: https://github.com/facebook/jest/issues/7136
- if two tests ran in parallel, `onwarn` was getting results from both, screwing up the `toBeCalledTimes` number
- couldn't get the syntax error to work with `toBeCalledTimes` either
- if no mock is given, it _does_ print warnings, but if a mock is given, it doesn't print, yet isn't called?
- I think the mock is getting screwed up by the error being thrown here; maybe improperly saved or something
|
||
|
Anton Gilgur
|
76109fcfe9
|
fix: missing tsconfig error shouldn't say undefined (#383)
- per the conditional above this line, `file` is falsey, so printing it doesn't make sense - per same conditional though, `pluginOptions.tsconfig` exists, so we can print that - fixes a test TODO/FIXME that had to workaround this bug as well |
||
|
Anton Gilgur
|
70724b1af5
|
fix(test): handle pretty formatting (#377)
- integration tests and `pretty` default change got merged simultaneously, so the tests need to be updated to handle the new `pretty` default - the coloration of the errors is quite different when using `pretty` - so instead of re-implementing TS's colors in the tests, just test a portion that is the same color: the error text - also TS doesn't specify semantic vs. syntax, that's something `print-diagnostics` does - this test is more resilient to change as well I suppose since no error codes or colors |
||
|
Anton Gilgur
|
61d78bd2f3
|
test: add initial integration test suite (#371)
* test: add initial integration test harness
- some additional ts-jest config is needed to parse `index.ts`
- since Rollup isn't used when importing files during testing, we need to enable `esModuleInterop`
- the import of `findCacheDir` was immediately erroring without this (and maybe more, but `esModuleInterop` made the import work)
- add `tsconfig.test.json` for this purpose
- use `ts-jest` JSDoc types to get types for the ts-jest config as well
- add an integration/ dir in the __tests__/ dir for all integration tests
- add a fixtures/ dir in there as well for integration test fixtures
- add a `tsconfig.json` for all fixtures to use
- basically so that rpt2 source code is not `include`d, though also bc this may be good to customize for fixtures
- add a simple fixture with "no-errors" that does an import and a type-only import
- add a simple integration test that just ensures that all files are part of the bundle
- the bundled `index` file and each file's declaration and declaration map
- and no _other_ files!
- for Rollup, need to use paths relative to the root of the whole project
- probably because that is `cwd` when running `jest` from root
- will probably abstract out helpers here in the future as the test suite grows
- 70% coverage of `index.ts` with just this one test!
- update CONTRIBUTING.md now that an integration test exists
* refactor: use `local` func for path resolution
- similar to the unit test suite
- also remove the `.only` that I was using during development
* add comparison test for cache / no cache
- should probably do this for each integration test suite
- refactor: split out a `genBundle` func to DRY things up
- this was bound to happen eventually
- don't think testing other output formats is necessary since we don't have any specific logic for that, so that is just Rollup behavior
- take `input` path and rpt2 options as params
- and add the `tsconfig` as a default
- add a hacky workaround so that Rollup doesn't output a bunch of warnings in the logs
- format: use double quotes for strings consistently (my bad, I normally use single quotes in my own repos)
* refactor: use a temp cacheRoot
- so we can clean it up after testing to ensure consistency
- and so we're not filling up the cache in `node_modules` with testing caches
- also add a comment to one the of the tests
* test: check w/ and w/o declarations + declaration maps
- make sure they don't get output when not asked for
* fix(test): actually test the cache
- `clean: true` also means that no cache is created
- which is a bit confusing naming, but was requested by a few users and implemented in f15cb84dcc99a0bd20f3afce101c0991683010b6
- so have to create the bundle one _more_ time to create the cache
- note that `tscache`'s `getCached` and `isDirty` functions are now actually covered in the coverage report
* fix(ci): increase integration test timeout
- double it bc it was occassionally failing on CI due to timeouts
* test: ensure that JS files are part of bundle too
- ensures that TS understands the JS w/ DTS w/o error
- and that rpt2 filters out JS while Rollup still resolves it on its own (since Rollup understands ESM)
- similar to testing w/ a different plugin (i.e. literally testing an "integration"), but this is native Rollup behavior in this case where it doesn't need a plugin to understand ESM
- also the first test of actual code contents
- reformat the cache test a bit into its own block since we now have ~3 different sets of tests in the suite
* optim(test): don't need to check cache each time
- this doesn't test a new code path and the first test already tests the entire bundle for the fixture, so the other tests just repeat that
* test: add initial error checking integration tests
- refactor: rename `integration/index.spec` -> `integration/no-errors.spec`
- refactor: split `genBundle` func out into a helper file to be used by multiple integration test suites
- simplify the `genBundle` within `no-errors` as such
- create new `errors` fixture just with some simple code that doesn't type-check for now
- add test to check that the exact semantic error pops up
- and it even tests colors 😮 ...though I was confused for some time why the strings weren't matching... 😐
- add test to make sure it doesn't pop up when `check: false` or `abortOnError: false`
- when `abortOnError: false`, detect that a warning is created instead
- due to the new `errors` dir in the fixtures dir, the fixtures `tsconfig` now picks up two dirs
- which changes the `rootDir` etc
- so create two tiny `tsconfig`s in both fixture dirs that just extend that one
- and now tests all run simiarly to before
* fix(ci): increase integration test `errors` timeout
- this too timed out, probably due to the checks that _don't_ error
* optim(test): split non-erroring error tests into a different suite
- so that Jest can parallelize these
- CI was timing out on this, so splitting it out should help as it'll be 10s each
* fix(ci): bump integration test timeout to 15s
- bc it was still timing out in some cases 😕
- this time the `no-errors` suite was timing out, so just increase all around
* fix(test): output a `.js` bundle, not `.ts`
- woooops... was wondering why it was `.ts`; turns out because I wrote the `output.file` setting that way 😅 😅 😅
- also add a missing comment in errors for consistency
- and put code checks _after_ file structure checks, since that's a deeper check
* test: check that `emitDeclarationOnly` works as expected
- should output declaration and declaration map for file
- code + declaration should contain the one-line function
- code _shouldn't_ contain anything from TS files
- since this is plain ESM code, we don't need another plugin to process this
- nice workaround to installing another plugin that I hadn't thought of till now!
* test: add a syntactic error, refactor a bit
- add a file to `errors` with a syntax error in it
- apparently this does throw syntax err, but does not cause `emitSkipped: true`... odd...
- so may need another test for that...
- `abortOnError: false` / `check: false` both cause Rollup to error out instead
- rename `errors/index` -> `errors/semantic` since we have different kinds now
- change the `include` to only take a single file, so that we don't unnecessarily generate declarations or type-check other error files
- refactor(test): rewrite `genBundle` in both files to take a relative path to a file
- simplifies the `emitDeclarationOnly` test as we can now just reuse the local `genBundle` instead of calling `helpers.genBundle` directly
- use the same structure for `errors`'s different files as well
- refactor(test): split `errors.spec` into `tsconfig` errors, semantic errors, and syntactic errors (for now)
- add a slightly hacky workaround to get `fs.remove` to not error
- seems to be a race condition due to the thrown errors and file handles not being closed immediately on throw (seems like they close during garbage collection instead?)
- see linked issue in the comment; workaround is to just give it some more time
- not sure that there is a true fix for this, since an improper close may cause indeterminate behavior
* fix(test): normalize as arguments to `include`
- got a Windows error in CI for the `errors` test suite
|
||
|
Anton Gilgur
|
9c508989b8
|
docs: mention module: "ES2020" compatibility (#376)
- it's already been supported since eb1dd17babde0b22e9540b12e671eb56d9d6bce0, but the docs and error message were not updated to mention it - so add both to make sure users aren't confused - also re-order it to be ES2015, ES2020, then ESNext consistently, which is their module order (c.f. https://github.com/microsoft/TypeScript/issues/24082) - modify test to account for the new error message - make it a bit more resilient to change by only testing a substring as well (c.f. https://jestjs.io/docs/expect#tothrowerror) |
||
|
Anton Gilgur
|
d078f3f577
|
clean: remove backward-compat checks for old Rollup versions (#374)
- we require Rollup `>=1.26.3` in the peerDeps and the README and have for years now
- (and Rollup is currently at `2.75.7`, so this is a pretty low target)
- as such, we can remove various checks that are legacy backward-compat remnants for old Rollup versions:
- `this.meta` was added to `options` in `1.1.0`: https://github.com/rollup/rollup/blob/master/CHANGELOG.md#110
- `this.addWatchFile` was added at least in `1.0.0`: https://github.com/rollup/rollup/blob/master/CHANGELOG.md#100
- `this.error` and `this.warn` were added to `transform` in `0.41.0`: https://github.com/rollup/rollup/blob/master/CHANGELOG.md#0410
- this simplifies some of the code a decent bit, `RollupContext` in particular
- see also the usage of `buildEnd` in my other PR
- modify tests to account for these changes; basically just simplify them
- and remove the check against private internals of the class, as they're private
- guess I forgot to remove this when I was refactoring the test code?
|
||
|
Anton Gilgur
|
63a644a762
|
clean(cache): remove unused checkNewCache parameter (#368)
- it is set to `true` in every usage, so just always check the new cache and remove the parameter entirely
- rewrite the comments to reflect this change
- and condense them a bit
- modify tests a bit to reflect this change as well
|
||
|
Anton Gilgur
|
67f1d86a96
|
refactor(cache): simplify noCache condition (#362)
- if there is no cache, we don't need to do any operations on a cache at all, so we can just totally skip all the cache operations and return early
- this should be a perf improvement, at the very least on memory usage, as lots of stuff isn't created or processed now
- this may make `clean: true` a more optimal choice for smaller projects, as the FS usage when writing to cache may be slower than the small amount of compilation and type-checking required in small projects
- to start, from the constructor, the only necessary piece when `noCache` is the dependency tree
- this is needed for `walkTree` and `setDependency` to function, but otherwise is unused when `noCache`
- this _might_ be able to be further optimized to remove the graph entirely when `noCache`, but that is saved for potential future work and not part of this commit
- so, we can just move the tree creation further up in the constructor as its previous ordering within the constructor does not actually matter
- once this is done, we can just early return when `noCache` instead of doing all the cache-related actions, since they're not neeeded when there is no cache
- no need to set `cacheDir` or any hashes etc since they're not used
- note that `clean` only uses `cachePrefix` and `cacheRoot`, which are already set, and does not use `cacheDir`
- no need to `init` the cache as it's not used
- also slightly change the ordering to move `init` right after its prereqs are done, i.e. setting `cacheDir`, `hashOptions`, etc
- just keeps with the flow instead of calling it in the middle of the ambient type processing
- no need to check ambient types as that is only used for cache invalidation (marking things dirty), which is not used when there is no cache
- note that `isDirty` is literally never called when `noCache`
- from there, since we don't call `checkAmbientTypes` or `init` when `noCache` (the constructor is the only place they are called and they are both `private`), we can entirely remove their `noCache` branches
- fairly simple for `checkAmbientTypes`, we just remove the tiny if block that sets `ambientTypesDirty`, as, well, "dirty" isn't used when there is no cache
- for `init`, this means we can entirely remove the creation of `NoCache`, which isn't needed when there is no cache
- that means we can also remove the implementation and tests for `NoCache`
- and the reference to it in `CONTRIBUTING.md`
- in `done`, we can also simply skip rolling caches and early return when there is no cache
- the only other tiny change is the non-null assertions for `ambientTypes` and `cacheDir`
- this matches the existing, simplifying non-null assertions for all the caches, so I did not workaround that
- _could_ set a default of an empty array for `ambientTypes` etc to workaround this, but thought it better to match existing code style and not add new things
- this also matches the behavior, as while `ambientTypes` and `cacheDir` could be `null`, this is only if there is no cache, in which case, they are never used
|
||
|
Anton Gilgur
|
d32cf839fa
|
refactor: simplify hosts to directly assign tsModule.sys where possible (#349)
- no need to duplicate types this way, which can and have changed over time -- it's always the same typings this way - also reorganize `host.ts` to have similar categories of functions near each other, instead of a mix of functions wherever - similar to how I organized the tests for `host` as well - shrink the code a bit this way too - add a comment about `getDefaultLibFileName`'s confusing naming pointing to the TS issues about how this is an old mistake but changing it now would be breaking - this is also how the TS Wiki recommends setting up hosts: https://github.com/microsoft/TypeScript/wiki/Using-the-Compiler-API#incremental-build-support-using-the-language-services - NOTE: because of how `tsproxy` works to support alternate TS implementations, this does require that `tsModule` _exists_ at the time of instantiation, i.e. that `setTypescriptModule` has already been called - for `host.ts`, `LanguageServiceHost` is only instantiated after `setTypescriptModule`, but for `diagnostics-format-host.ts`, it is immediately instantiated (at the bottom of the file), hence why `getCurrentDirectory` can't just be assigned to `tsModule.sys` - there is a way to fix this, but the refactoring is more complex as it would require creating in `index.ts` and then passing it as an argument -- would want to refactor more at that point too, so leaving that out for now in this otherwise small, isolated refactor - for a different, but related reason, the `host.trace` tests have to mock `console` instead of just `console.log`, since `trace` would just be set to the old, unmocked `console.log` otherwise - as it's assigned directly to `console.log` now |
||
|
Anton Gilgur
|
b44b069cb2
|
fix: force noEmitOnError: false (#338)
- `noEmitOnError: true` acts like `noEmit: true` when there is an error
- this is problematic because it will then cause _all_ files to have
`emitSkipped` set to `true`, which this plugin interprets as a fatal
error
- meaning it will treat the first file it finds as having a fatal
error and then abort, but possibly without outputting any
diagnostics what-so-ever as the file with the error in it may not
yet have run through the `transform` hook
- i.e. an initial file that imports an erroring file at some point
in its import chain will cause rpt2 to abort, even if that
initial file _itself_ has no type-check/diagnostic issues
- bc TS does whole-program analysis after all
- this has only been reported as an issue once so far, probably because
it defaults to `false` in TS and, as such, is rarely used:
https://www.typescriptlang.org/tsconfig#noEmitOnError
- we usually have the opposite issue, people trying to set it to
`false` (i.e. the default) because they don't realize the
`abortOnError` option exists
- add `noEmitOnError: false` to the forced options list and tests too
- add it to the docs on what tsconfig options are forced
- and add a reference to the issue like the existing options
- also reference `abortOnError` since they're commonly associated with
each other and that plugin option is often missed (per above)
- briefly explain that `noEmit` and `noEmitOnError` are `false` because
Rollup controls emit settings in the context of this plugin, instead
of `tsc` etc
- should probably watch out for when new emit settings are added to
TS, as we may want to force most with the same reasoning
|
||
|
Anton Gilgur
|
8e580375e1
|
fix: add realpath to host to properly resolve monorepos (#332)
* fix: add `realpath` to host to properly resolve monorepos
- tested this in a pnpm repo with symlinked deps and it worked there,
so I believe this fixes all pnpm issues
- it may also fix some Lerna issues if they were due to symlinks, but
I didn't check those
- not sure about others, e.g. Rush, Yarn workspaces, Yarn PnP
- I figured out this was needed by staring at the TS source code and
then I found this line:
|
||
|
Anton Gilgur
|
d318e9a8fc
|
fix: normalize paths in get-options-overrides (#331)
- the `outDir` was not normalized after the `/placeholder` part was
added to `cacheRoot`
- `cacheRoot` could have `\` directory separators on it on Windows,
which caused some tests to fail on Windows before
- tests have been normalized now too
- `expandIncludeWithDirs` used `path.join` without normalizing after
- `path.join` uses the OS's native separators (`posix.join` would do
POSIX separators only), so when the paths were already normalized
and then `path.join`ed, this would cause mixed separators on Windows
- this fixes the current CI failure on Windows in the `createFilter`
tests (`rootDirs` and `projectReferences`, which use
`expandIncludeWithDirs`)
- c.f. https://github.com/ezolenko/rollup-plugin-typescript2/runs/6516149780?check_suite_focus=true
|
||
|
Anton Gilgur
|
e8240ae505
|
test: 100% coverage in get-options-overrides (createFilter) (#329)
* test: 100% coverage in get-options-overrides (createFilter)
- get the skipped test to work by using absolute paths, as that is what
the `filter` function expects (and is how it is used in this codebase)
- use the same helper func on the main `createFilter` test as well to
ensure that we're consistently testing the same paths
- (though `**/*` basically matches _everything_)
- add a test for project references as well
- and remove the `**/*` from the include for this so it doesn't match
everything
- this also tests the single string include code path as well
- add a test for the `context.debug()` statements as well
- couldn't get to 100% Funcs coverage without this
- used a simplified mock instead of actually using `RollupContext` so
that this doesn't rely on that file/unit to work
* quick fix for Windows?
|
||
ezolenko
|
529c8530f5 | - fix for linter | ||
|
Anton Gilgur
|
60f3489e87
|
deps: upgrade tslib to 2.4.0, remove @yarn-tool/resolve-package (#326)
- tslib 2.4.0 is forward and backward-compatible with older and newer
Node exports mechanisms, so the Node 17 error should no longer be
present
- it has the older `./` and the newer `./*` in its package exports,
which should allow for `package.json` to be read in both older and
newer implementations
- this allows us to remove the extra dep on `@yarn-tool/resolve-package`
as well
- other than less unnecessary deps being good,
`@yarn-tool/resolve-package` is also a not well-documented package
with very few users, which does not make for a good security posture
for rpt2 (which has historically prioritized supply chain security
in other issues around deps) or, in particular, its consumers, which
there are very many of (in contrast with `@yarn-tool`)
- per my issue comment, we could also have avoided the extra dep prior
to the tslib upgrade by resolving to absolute paths, as Node only
does a "weak" encapsulation of relative imports
- test: add a small unit test for tslib.ts to ensure that this method
works and passes on different Node versions in CI
- more a smoke test that it runs at all, the testing is additional
and a bit duplicative of the source tbh
|
||
|
Anton Gilgur
|
56716def97
|
test: add initial unit test suite (#321)
* some unit tests
modifications made by agilgur5 to brekk's original commit:
- rebase with `master` that is 3 years newer
- fix conflicts with newer code, esp the `tsModule`/`tsProxy` changes
in this commit
- move `jest` config to the bottom and don't reformat `package.json`
with tabs
* more
modifications made by agilgur5 to brekk's original commit:
- fix merge conflicts with code that is 3 years newer
* fix tests for windows
* flip that test
* deps: upgrade Jest to v28
- and ts-jest to v28
- @types/jest doesn't have a v28 yet
- look ma, no more vulns!
* refactor: split jest config to jest.config.js
- and add JSDoc typings to it
- per https://jestjs.io/docs/configuration
* clean: remove redundant jest config
- most of these options are either unused (like `modulePaths` etc) or
covered by Jest's defaults (`moduleFileExtensions`, `testMatch`, etc)
- also use `ts-jest` preset per its docs and migration docs
- https://kulshekhar.github.io/ts-jest/docs/migration
* refactor: move tests into __tests__ dir
- per ezolenko's request, though I also prefer to separate them
- fix all relative imports to use `../src/` now
formatting changes:
- improve ordering of imports in tests -- externals first, newline, then
internal imports
- do this **consistently**
- consistently put spaces around braces in destructured imports
- really need better linting (and update to ESLint)
- consistently add newline after imports and before declarations
- in general, add more newlines for readability in several files
* deps: import from @jest/globals instead of using globals
- no config needed and uses standard imports too
- also required for future Jest ESM support I believe
- I created the `jest-without-globals` package before this feature was
released in Jest core and contributed a PR for it too, so might have
a bias toward not using globals
* fix(test): update get-options-overrides spec to match latest file
- basically to match the changes I made when fixing declaration map
sources ~1.5 years ago in ec0568ba2c8e206372f94164e697b1469bf3f33d
- also to add `cwd` to options
- fix: use `toStrictEqual` everywhere, in order to actually check
`undefined` properties, as this is necessary for some of the checks
- I believe the original author may have intended to actually check
`undefined` given that they wrote it in the test, but maybe did not
know to use `toStrictEqual` instead of `toEqual`
* refactor(test): use obj spread in get-options-overrides
- instead of re-writing the options multiple times, write up some
defaults and use object spread
- and refactor `makeDefaultConfig()` to just use
`{ ...defaultConfig }` instead to match the rest
- re-write normalizePaths to be a `for ... of` loop instead of using
`Array.map`
- simpler / more straightforward than a function with side-effects
- feat: add typings in several places
- that's why we're using TS and ts-jest right??
* fix(test): update host spec to match latest source
- LanguageServiceHost now requires 3rd argument, cwd, so add that to
all usage
- add afterDecalarations to transformers
- Jest's test.skip can now use async/await, so do that
- it was also giving a type error, and this is simpler anyway
- change expected to use `__tests__` directory now
- fix: use `expect.arrayContaining` for `host.getDirectories` test as
there are more temp directories that get added here, such as
`build-self` and `coverage`
- in general, might be less fragile if this used a generated directory
instead of an actual one
- but save bigger refactors for later
* refactor(test): use async/await, remove truncate func in host spec
- instead of using Promises with `done()`, use async/await for clearer
code and less indentation too
- remove the `truncateName` function as it's not needed; `local` will
result in the same name
- remove commented out test code
- this seemed to just be there for testing the code, and not
ready-for-production comments
* refactor: use consts in host spec instead of locals
- basically using pre-defined fixture vars instead of ones defined
inside the test
- which is more straightforward, easier to read, and less fragile
- shorter names too
- also use a __temp dir for auto-generated files instead of creating
them in the same dir and confusing the editor file tree and potential
watchers
- change jest config to make sure only spec files are being watched
- gitignore __tests__/__temp/ dir in case tests crash etc
* fix(test): update rollupcontext spec to match latest source
- use PluginContext and IContext types instead of `any`
- we're using TypeScript right??
- add `debug` level logging in a few places where it was missing
- update stubbedContext to have latest PluginContext properties
- watcher isn't in there anymore and a few new properties were added
- fix type errors with stubbedContext
- give it an intersection with IContext for `info` and `debug`
verbosity levels
- force the logging funcs to `any` as they don't quite match the
Rollup types
- force them to `any` when deleting them as well because they're not
optional properties
- Note: would be good to not force so much `any` if possible, but this
may be difficult without more advanced internal Rollup mocks
- couldn't find any testing packages for this :/
- test: add verbosity expect for debug too
- refactor: context2 -> context
- there wasn't another one, so just use the same name consistently
- I'm guessing there was another one at some point in the
development of this and then it was removed but not renamed
* lint: add __tests__ to lint dirs, fix lint errors
- surprisingly only in jest.config.js?
- really need to update to @typescript-eslint ...
* ci: add unit tests to matrix
- run after all the builds for now
- it can be done in parallel as a separate job, but then have to
duplicate the NPM install etc, so may not be optimal that way
- refactor: add test:watch and test:coverage scripts
- shortcut scripts so don't have to `npm test -- --coverage" etc
- also remove `--verbose` from `test` as that's not necessary
* test: add unit tests for createFilter
- increases coverage of get-options-overrides significantly
- couldn't figure out `rootDirs` -- either the code or my tests are
wrong, so just skip that one for now
- refactor: move makeStubbedContext etc into a shared fixtures dir so
that it can be used for both rollupcontext tests _and_
createFilter tests
- in my stylistic opinion, I prefer to put nearly _all_ consts like
these into fixtures dir
- configure jest to ignore test helpers in coverage reporting such as
fixture files
- format: '' -> "", add semicolon in some places
- I use single quotes and no semicolons, so missed that in a few
places
- lint didn't check for that and no prettier auto-format :/
* refactor: use consts, async/await, etc in rollingcache spec
- basically using pre-defined fixture vars instead of ones defined
inside the test
- which is more straightforward, easier to read, and less fragile
- shorter names too
- left a couple of ones as is where they were only used once very
quickly -- could make them fixture vars too but 🤷
- use async/await instead of Promises with `done()` etc
- also use more `fs-extra` functions that support Promises instead of
synchronous `fs` functions (e.g. `existsSync` -> `pathExists`)
- async should be a small optimization for tests too
- fix: use __temp dir for auto-generated files instead of creating
them in a fixtures dir and breaking actual fixtures
- format: a few multi-line statements were able to be condensed to a
single line, so do so
- esp as multi-line was not helping readability since this is just
irrelevant test data (may have hampered readability actually)
* docs: add a testing section to CONTRIBUTING.md
- goes over the different ways of running the tests (watch + coverage)
- line about unit and integration tests was moved into this section
- and altered to reflect the current state of the repo now that a good
amount of unit tests exist
- also move "Linting and Style" into its own section with a list
- move "fix any failed PR checks" to the top as overall guidance on
making a PR
- previously it was in the middle of linting and style, which felt a
bit disjointed (maybe made sense earlier before builds were in CI?)
- name the last section "Building and Self-Build"; leave content as is
- well mostly, change the first line as "fastest way to test" is not
necessarily accurate anymore now that there are actual tests
* fix(test): undo tsTypes -> tsModules changes in source code
- original author, brekk, had made these changes, likely because without
them, the tests would throw in the source lines where `tsModule` was
used with things like "Cannot read property 'ModuleKind' of undefined"
- the actual fix for this is to instead use `setTypescriptModule` from
tsproxy as this is how it is used in the source
- it's mainly needed for when an alternate TS is substituted
- brekk probably didn't know the codebase well enough to know that
- add `setTypescriptModule` to all specs that need it
- 100% test coverage of tsproxy now too!
- this should hopefully fix the build errors we were getting as well
* test: get host spec customTransformers test working
- it seemed incomplete and that's why it was skipped, no comment there
stating otherwise, so just modified it a bit to get it to work and
pass properly
* ci: add Node 14, 16, 18, and macOS, Windows to matrix
- drop Node 10 testing
- Node 10 went EoL a while ago and was causing the latest version of
Jest to throw an error
- still test Node 12 since it only went EoL recently, but could drop it
as well if wanted
- may want to set `package.json#engines` or update min requirements in
the README if so, though it likely still works on older Node, we
just can't easily test it
- add macOS and Windows testing to the matrix since TS and Rollup both
behave differently on different OSes, in particular with regard to the
filesystem
- POSIX paths on Windows, case-insensitivity on macOS
- give the job a name that appears in the PR checks and Actions tab
* refactor: use __temp dir in options-overrides spec
- similar to previous commits, don't use the actual fixtures dir, use
the `__temp` dir and a subfolder within for this spec file
specifically so as to not interfere with other tests
* test: 100% coverage for host.ts, refactor host.spec.ts
- test: get the skipped `readFile` test to work
- `ts.sys.readFile` definitely returns stuff, it's used for snapshots
- it wasn't working because it was testing the wrong extension,
`.js` instead of `.ts`
- I thought this was intentional when I first added consts here, but
turns out that was actually a mistake by the original author
- refactor: merge the `readFile` tests with the main test suite
- first I merged them together, but as they were just one-liners, I
merged them with the first set, since it wasn't particularly
different from those
- refactor: split up the main test suite into a few commented sections
- core snapshot functionality, fs functionality, misc
- a lot less random now
- refactor: use `getDirectories` on the testDir instead of project root
- much less fragile to use the dir generated here, and no hack-ish
`arrayContaining()` either
- refactor: one-line the case-insensitive test
- that simplifies it a lot!
- refactor: use fixed consts for the repetitive default config
- and use a `testOpts` variable for less fragile testing
- test: add tests for version 2 branches of snapshot funcs
- these weren't tested before so it had only tested the `|| 0` branch
of the conditional
- also actually test `reset` instead of calling it at the beginning
- refactor: no more "dirty checking" of instances' private interfaces
- we're testing it all through the public interfaces instead!
- test: add a simple test for the new `trace` method
- test: coverage for `after` and `afterDeclarations` transformers
- test: coverage for different `undefined` states of transformers
- refactor the two test suites as one for `undefined` and one for
all 3 transformers
- and put the working one first for naming purposes
- refactor the `setLanguageService` call as a simpler type coercion
- remove the transformer interactions in the main test suite
- yea, this is a big commit, but I was refactoring everything anyway and
this is probably gonna be squashed anyway
* refactor: remove remaining "dirty checking of instance as any"
- no need to check private interfaces in these remaining tests, the
functionality itself is already tested via the public interface
* fix(test): get tests working on Ubuntu and Windows
- host is only case-sensitive on Linux
- prev was everything except Windows; so flipped the conditional a bit
- host and TS itself use "/" normalized paths, so use those
- and add a hacky workaround for a TS bug I managed to find
- formatHost will use OS separators, so no "/"
- make the test more accurate and less fragile by comparing to
`process.cwd()` as well -- which should normalize on Windows
* refactor: use consts and shrink check-tsconfig
- think it's a bit easier to read this way, it's only 3 tests after all
* refactor: remove normalizePaths func from get-options-overrides spec
- this isn't necessary and added some complexity (and verbosity)
- only one dir that needs normalization anymore, and that's just the
cache dir, so just add it to the const/var fixture instead
- shrink the code a bit as a result too
- and use a bit different code style to shrink it too
- annnd found a Windows bug in get-options-overrides.ts here too...
- leave a `// TODO: ` comment here to fix the source as this PR is
solely focused on tests (i.e. no source changes)
Co-authored-by: Brekk <brekk@brekkbockrath.com>
Co-authored-by: bbockrath <bbockrath@goodgamestudios.com>
|