* fix: preemptively `filter` type-only files to workaround Rollup bug
- basically, right now, the addition of `this.load` on all `references` is causing Rollup to error out on JSON files
- specifically, this is impacting `configPlugin` usage (i.e. `rollup.config.ts`), where previously one didn't need `@rollup/plugin-json`, but now it is erroring out without it
- I tracked this down to be because of `this.load` specifically
- to avoid this and similar such issues, we can preemptively `filter` out files before calling `this.resolve` / `this.load`, which should end up `exclude`ing JSON files and any other non-rpt2 files
- this should also make it a bit more efficient to skip some recursion
- and non-rpt2 files shouldn't include any type-only files
- confirmed that this change fixes the error
- and that the type-only tests still pass
* refactor: use a common function for resolution checks
- since the same logic is used in `resolveId` and these _should_ be equivalent
- in the future, we might want to add more common logic to this function, e.g. `getAllReferences` removes `undefined` and uses `moduleNameResolver` as well, similar to `resolveId`
- may not be so easy, so TBD
- for instance, even moving the `undefined` check into the func required adding a type guard, as the compiler wasn't quite able to infer that passing the func meant it was not `undefined`
* feat: also ignore `.d.cts` and `.d.mts` file extensions
- support newer TS extensions
- rpt2 should _always_ ignore declarations
- regardless of the `exclude`; as in, if a user accidentally removes declarations in an override, rpt2 should still not directly read declarations
- as they are normally read ambiently by TS and not _directly_ by Rollup or TS
* fix comment -- there's more reasons why we shouldn't resolve all references
- we don't `return false` in `resolveId`, so any new file that wasn't previously in Rollup's pipeline _must_ be resolved
- `return` just defers to the next plugin, so, for a declaration, it eventually causes Rollup to try and fail to resolve on its own, giving an `Unexptected token` error message
- but we _don't_ want to `return false` in `resolveId` if they _intentionally_ imported a declaration for some reason (e.g. if they're going to transform it in some way)
- if we did `return false`, no other plugin could process either
- so as a result, we should just never call `this.resolve()` on anything we don't expect to be able to resolve
- i.e. don't add anything new to the pipeline that we don't resolve ourselves
* fix: don't resolve `filter`ed files
- if they're `exclude`d / not `include`d, then we shouldn't be processing them
- we're already not transforming them, so this just applies the same exclusion to resolving
- this is _partly_ a regression from b0e39228b639229ce29d5d8d0b4bd236e4b20852, as that removed the `allImportedFiles` Set that previously filtered out files not in the `tsconfig` `include`
- but that _itself_ was a regression that was removed -- files that didn't pass `filter` should have _never_ been resolved
- basically, the `allImportedFiles` regression was covering up this long-standing bug
- also move `.d.ts` check to above the `filter` check
- we shouldn't be adding declarations to the `cache`, in particular as we don't process declarations, so they'll never be marked as dirty
- having this check above the `filter` should be slighltly more efficient as well (as would not having these files in the cache graph)
types: be more specific with `filter`'s type
- no need for this to be `any`
* build
- patch release has been waiting for a few weeks
* pub: release v0.34.1
- patch bump with the past few fixes
- bump internal rpt2 version to 0.34.0
* hotfix: `this.load` was added in Rollup `2.60.0`; don't run on older versions
- this is my bad, I didn't realize `this.load` came out _much_ later than `this.resolve`
- c.f. https://github.com/rollup/rollup/blob/master/CHANGELOG.md#2600
- since we're backward-compatible w/ Rollup `1.26.3`, add in a version check for the type-only fix
- the type-only fix will error out in Rollup versions less than `2.60.0`, so instead of erroring out, just skip this functionality
- TODO: should move back to using `semver` that was removed in 08d2f5bcad7cb9a9a0974f3707a27d29dd89da7c
* deps: add back `semver` and use for version checking
- reverts the `semver` removal in 08d2f5bcad7cb9a9a0974f3707a27d29dd89da7c
- add back the TS version range check
- add a Rollup version range check, fixing an old issue
- check `this.load` Rollup version with `semver` instead of string comparison
- add `semver` to `external`s list
- also remove `resolve` as it's no longer used as of 74f6761ff63863cdb3e92ca1f33640fe6f4fc7db
- and re-organize the list so that Node built-ins are in one section while deps are in another
- makes it clearer what we're marking as external
* dx: add a nice warning when `this.load` is not supported
- so it doesn't just silently skip the type-only fix on Rollup versions <2.60.0
* fix(test): add globals for strings that are replaced during build
- `TS_VERSION_RANGE`, `ROLLUP_VERSION_RANGE`, `RPT2_VERSION` were previously only defined during builds, so would cause tests to fail
- add these vars as (namespaced) globals in the Jest config so that they can be used in tests too
- if they don't exist on `global` (or if `global` doesn't exist), then use the built strings instead
- see in-line comments for more details
- also reorder `re` placement to match the usage order in the code
- and fix lint issues (`no-string-literal`)
Co-authored-by: Eugene Zolenko <zolenkoe@gmail.com>
- 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
* 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
- we have release notes already in GH releases, so add a simple `CHANGELOG.md` that references it
- I use something similar in most of my libraries
- this should hopefully make it easier for users to find the release notes in case they hadn't seen them on GH before
* 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
- as everything is created in the `buildStart` hook now (which has `RollupContext`), we can create the `cache` there too
- no need for slightly hacky, lazy creation during `transform` anymore
- simplifies it and also standardizes it so it's created the same way as all the other instance vars
- fix: reset `cache` after each watch cycle
- previously the cache was never reset, meaning that if anything became dirty in a watch cycle, it would never get reset back
- in some cases, this would mean that the cache was effectively always dirty during an entire watch mode run, and therefore never used
- this would be quite inefficient as the FS usage for the cache would just go to waste
- see that test coverage has now increased as a result!
- 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
- `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
* 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
- 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
- when `options` is called, `generateRound` should have already been reset to `0`, so this is redundant / unnecessary
- `options` is only called once per watch cycle
- `options` is also an `input` hook, not an `output` hook, i.e. it's only called once for _all_ outputs, _not_ per each output
- `generateRound` only tracks the output round
- this might be leftover historical remnants prior to Rollup officially separating `output` hooks, but even before then, it should only have been called once for _all_ outputs
- since, per the Rollup API, it's only called during `rollup.rollup` and not during `bundle.generate` etc
- c.f. old docs https://github.com/rollup/rollup/blob/v1.18.0/docs/05-plugin-development.md#options
- it's possible this is _even older_, but I couldn't find plugin docs for Rollup pre-1.0 to try to confirm against
- 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
- `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)
* 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>
* 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
- 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
This reverts commit e145d0baebbbef1eec691b99d1ce0dce1e49f38c.
- Per discussion on the original issue and reverted PR, it seems that the request to use `.d.ts` instead of `.vue.d.ts` was made in error
- `.d.ts` seems to only be necessary if Vue users were importing `.vue` SFCs without extensions ("extensionless")
- i.e. `import MyComponent from "./MyComponent"` instead of `import MyComponent from "./MyComponent.vue"`
- and "extensionless" imports are no longer supported by the Vue team (but used to be)
- requiring extensionless imports also breaks imports when extensions _are_ used
- so these are not necessarily compatible with each other, but the Vue team support strongly suggests that `.vue.d.ts` would be the proper way forward
- reduce the complexity of the code by decreasing the amount of nesting
- note that `print` returns `void` anyway, so calling it within the `return` statement doesn't change anything
- and this is within a `void` `forEach` at that too
- just slightly simplify / DRY up repeated code
- this matches the style of some of the other functions in the codebase, such as `emitDeclaration`, `getAllReferences`, etc
- also fix indentation in `typecheckFile`
- apparently I double-intended this accidentally, so make it single indent
- since the `buildEnd` hook exists nowadays, we can just reset it there, right before `generateBundle` is called for each output and increments it per output
- also modify the comment to account for this change and the fact that `buildEnd` exists and is used nowadays
- we can re-use the same one during watch instead of resetting it in the `options` hook
- should make the LS a bit faster / more efficient in watch mode as most source files are shared between watch cycles
- a few issues have reported that "failed to transpile" is a vague / confusing error
- and `emitSkipped` actually _doesn't_ mean that it failed to transpile, as, in current versions of TS, it's not due to syntactic or semantic errors
- so explicitly say "Emit skipped" instead, which is slightly less vague, in that it can actually be used as a search term
- and provide a link to my TS issue that lists some reasons why `emitSkipped` occurs, since this is unfortunately otherwise undocumented by TS
- in the future, hopefully that issue will be resolved and we'll be able to give better or more specific error messages, but for now this is probably the best we can do due to its undocumented nature, unfortunately
- follows the style of most of the other errors in the codebase
- when referencing a variable, file path, etc, put quotes around it in the output
- so it's easier to distinguish in the log
- the docs for `clean` don't seem to have been updated with f15cb84dcc99a0bd20f3afce101c0991683010b6, which slightly changed how it works
- also clarify that "wipes out cache" means it deletes _all_ previous caches by saying "wipes any existing cache" instead
- fix the `include` default explanation to mention `.tsx` files as well, since the regex includes those
- also add code backticks and capitalize TypeScript etc (guess I missed this option in my previous PRs? or I intentionally left it as a separate change a while ago...)
- also add code backticks around `node_modules` in `cacheRoot` as well while at it
- use non-rendered newlines between sentences for markdown style consistency
