- as I've done in other places, simplify a condition by using optional chaining syntax (over the previously necessary syntax with `&&` etc)
- also add a new line to the first `.map` here
- for style consistency and because this is a super long line
- this wasn't possible before the lodash refactor because it was all one big single statement in the chain
* refactor(cache): makeName -> createHash for clarity
- I've actually been confused multiple times as to what `makeName` does when I read through the cache
- I re-read the code and then am like "oh it's the hash"...
- so thought renaming it to `createHash` would make things **a lot** clearer
- `Name` -> `Hash`
- `make` -> `create` because that's the more common terminology in programming
- also rename variables that reference `makeName`'s return from `name` to `hash`
- for the same reason around clarity -- this way it's quicker to interpret whenever you see it too
- not to mention, `name` can be confusing since we also have `id`, which is a path that is very similar to a name too
- and lots of `fileName`s too
- so good to disambiguate/differentiate a bit
* rename object-hash default import so no shadowed variables
- use typedoc for `isDirty` comment so that it actually appears in IDEs etc
- and use `@returns` typedoc annotation for better specification
- modify the comment a little for better grammar: 3 "or"s and no commas -> 1 "or" and commas
- use more accurate terminology: "global types" -> "ambient types"
- that's also more consistent in this codebase itself, where there's a legit function named `checkAmbientTypes`
- in `init`, specify that the `RollingCache` error should never actually happen
- I was a bit confused by when this would happen, then checked the constructor, and the answer is basically never: it's an invariant / redundant error-check
- also add a new line for style consistency
- this should help make reproductions easier as it gives a quick
starting point requiring no boilerplate and is easily accessible from
a browser too
- decided to go with StackBlitz over CodeSandbox and repl.it as it's
WebContainer tech allows for running Node projects entirely _inside_
the browser (via WebAssembly)
- meaning that unlike CodeSandbox and repl.it, their platform doesn't
need to spin up and connect to a container on the backend
- so it's much more efficient, performant, and cost-effective
- and, importantly, it means you don't need an account to get started
with a Node project; you only need an account to save your work etc
- this makes a noticeable UX difference, since user funnels
generally decrease the more steps you take (especially a larger
one like creating an account)
- and we want reproductions to be as easy as possible to create
(so that people actually create them), so optimizing this flow
is important
- that being said, CodeSandbox and repl.it have much stronger OSS
presences and have open-sourced most of their core technology
- and Rollup and rollup/plugins use repl.it for reproductions:
https://replit.com/@rollup/rollup-plugin-repro
- so wanted to use those, but the UX difference was pretty
significant
- repl.it also didn't have as good a DX IMO
- have already made several reproductions of issues using this
environment as well, so can confirm that it works well!
- StackBlitz: https://stackblitz.com/edit/rpt2-repro
- GitHub: https://github.com/agilgur5/rpt2-repro
- add global version of TS as well
- with some frequency, people report not having the same issue with
`tsc`, and in some cases, that's because they're using their
globally installed version of `tsc`, which is at a different version
than their project's `tsc`
- this will help guard against that
- add OS and CPU details
- OS bc non-POSIX path separator issues with Windows are somewhat
common
- CPU in case certain architecture may have an impact, e.g. new Apple
ARM Silicon in Mac M1/M2 etc
- it really shouldn't as this is mostly a high-level library, but
some of the lower level functionality of TS and all the FS details
_could_ potentially have an impact
- add Node, Yarn, and NPM versions
- since sometimes it is an issue with the Node version or due to the
specific package manager
- e.g. Yarn workspaces, Lerna, `pnpm` symlinks etc have caused
issues before
- that being said, this command won't tell us _which_ of the package
managers the user is using, just all the installed ones' versions
- also wanted to add `pnpm` but it is not yet supported by `envinfo`
- there has been a PR out for it for a while, but hasn't been merged
by the creator :/
- thought that as it's somewhat? fresh in my head (pending if you
consider years of sporadic contributions), it would be really good to
document for new contributors who don't know where to start
- other than being good practice, thought this could be useful for a
number of reasons:
1. helpful resources like the TSConfig Reference, the TS Wiki, and the
Rollup Plugin docs, that I read through with some frequency
- (and contribute lots to the TSConfig Reference too, it's by far
the most useful resource in the ecosystem, IMO)
2. explaining directly in the docs how sparsely documented the TS
Compiler API is
- like it really makes things difficult and every time I'm looking
to contribute a bigger bugfix I look at the TS Wiki and don't
get a super helpful answer there
3. this codebase is actually fairly simple (it's the main reason I
became a contributor, as I've stated in a few issues) and truly not
that hard to get started with
- but there are some rabbit holes you can go down that can scare
away some contributors, like the cache code or the logging nuances
- so this gives a bit of a guide to not fall into some rabbit
holes and start off with the less complex bits of code
4. encourage more contributors to make some PRs like I first did!
- these steps should help in 2 ways:
1. hopefully avoid common misconfigurations from being repeatedly
opened as bug reports
- some of these have labels on them now to get a sense of how often
they pop up
2. help contributors and maintainers diagnose issues quicker and
figure out if an issue is indeed a bug
- the diagnosis questions are really common things we ask in an
issue already, so think it would work out better to get that
up-front
- we have labels for some of these too
- especially as issues can often get no response / go stale, and
if it's truly a bug, it's better to know that earlier than later
- condense the `CONTRIBUTING.md` a bit now that some of the most common
debugging steps are in the issue template
- didn't put in `npm prune` or `clean: true` _yet_, as don't want to
make the issue template _too_ big overnight
- those also aren't as common an issue and people seem to figure
out that an issue is due to caching bugs pretty often already
- probably want to move to GitHub's new beta Issue Forms moving forward,
but thought it'd be best to get the details into Markdown first,
_then_ can migrate
- and also creating Issue Forms seems to require moving to the
multiple Issue Templates format where users select a "type" of issue
(e.g. bug report vs. feature request etc), but we don't have
multiple templates yet, so that could be confusing
- 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
- my previous commit was mostly focused on index.ts, this one is for
get-options-overrides -- in general, I'm just trying to make old code
more readable as I come across and then realize that the code can be
re-written better, or, in this case, a conditional inverted
- these should run through the same `filter` as runs in `transform` etc
- prior to this, the plugin `exclude` would filter files in
`transform`, meaning no JS would be output for them, but would still
output declarations for these very same files that no JS was
produced for
- (this would only happen if one were using an `include` glob or
something that made the file appear twice, i.e. once by Rollup
in `transform` and once in `parsedConfig.fileNames`)
- this change makes it so the plugin `exclude` affects both JS
and DTS output equivalently
- it was very confusing when it didn't, and users relied on setting
different `tsconfig` `exclude`s to workaround this (as a
`tsconfig` `exclude` would make the file not appear in
`parsedConfig.fileNames`)
- this checks if any of the files in `parsedConfig.fileNames` are _not_
in `allImportedFiles`, but all the files in `parsedConfig.fileNames`
are explicitly added in the `options` hook on line 98
- so this is redundant / dead code; the check will never be true
- this can be considered a remnant of an old bug as an old commit fixed
a bug with `allImportedFiles` after it was released:
f24359e668
- previously, while errors would be printed, the flag for `noErrors` was
not set to `false`, and so there would be no yellow warning about
errors
- this is mostly just fixing asymmetric UX, but personally, I actually
have missed this error before myself, so maybe this will help
alleviate that
- the snapshot is set above, so no need to get it
- also the snapshot from above is used below already, so this doesn't
seem to be intentional
- it originates from 2d330640316894752bae5ae64d94bc90652e4564 which
similarly doesn't seem intentional
- and build
- v21 broke `graphlib`'s internal usage of `lodash` with its breaking
change, and v22 has another breaking change to fix that
- notably, this was breaking rpt2 imports for some users because rpt2
bundles in `graphlib` etc, meaning the nuances of the bundling got
leaked to consumers, and, in fact, broke their builds
- another upside to unbundling
- `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
- basically, general format is:
```ts
import x from "external-dep"
import y from "./internal-dep"
```
- so external deps, new line, then internal/local deps
- with some further sorting within there, like trying to keep Node
built-ins (e.g. `path`) at the top half of externals, then core deps
like `typescript`, then any other external deps
- and similar for internal deps -- core internals at the top half of
internals, then any other internal deps
- just to keep things consistent between files -- makes the top
easier to read through when it's similar between files
- also makes it easier for contributors to understand where to put
imports, as there's a sorting already there
- this is how I generally sort my imports and how I wrote most of the
unit test suite's imports as well
- there is automation for this that we should probably add once TSLint
is replaced here; some previous art:
- https://github.com/trivago/prettier-plugin-sort-imports
- https://github.com/lydell/eslint-plugin-simple-import-sort/
- Older:
- https://github.com/renke/import-sort/tree/master/packages/import-sort-style-module
- https://github.com/mcdougal/js-isort
- inspired by Python's `isort` ofc
- when using rpt2 as a configPlugin, there is no Rollup `output`, so it
will be `undefined`
- when `declarationMap: true` in `tsconfig`, this block of code is
called as a workaround due to the placeholder dir we must use for
output -- but, in this case, it errors out, since the
`declarationDir` is `undefined`
- `path.relative` needs a `string` as a arg, not `undefined`
- so skip this transformation entirely when there's no `output`, as
it doesn't need to be done in that case anyway
- this transformation code happens to have been written by me 2
years ago too, so I had fixed one bug with that but created
a different bug 😅 (fortunately one that only I have stumbled upon)
- a few `if (cond) { big block } return` could be inverted to
`if (!cond) return` then the block unindented instead
- generally speaking, this makes it a lot more readable (less
indentation, etc) and follows the existing code style in much of the
codebase, where it's a few quick one-line ifs and then just the rest
of the code
- shorten the resolvedFileName conditional by using optional chaining
- prefer the simpler `x?.y` over the older `x && x.y` syntax
- add a `resolved` variable for less repetition of the whole statement
- add a comment to the `pathNormalize` line about why it's used in that
one place and link to the longer description in the PR as well
- shorten comment about `useTsconfigDeclarationDir` so it doesn't take
up so much space or look so important as a result
- and it's easier to read this way and I made the explanation less
verbose and quicker to read too
- remove the `else` there and just add an early return instead, similar
to the inverted conditionals above
- similarly makes it less unindented, more readable etc
* - fix for vue declaration file names
* fix: use .d.ts instead of .vue.d.ts for Vue declarations
- `.vue.d.ts` was recommended by the `rollup-plugin-vue` maintainers
back when this code was originally written, but apparently it causes
problems with some imports
- plain `.d.ts` seems to work fine according to users
- and plain is the standard in TS too I believe
Co-authored-by: ezolenko <zolenkoe@gmail.com>
* 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:
67673f324d/src/compiler/moduleNameResolver.ts (L1412)
- it expects `host.realpath` to be implemented for TS's `realPath` to
work correctly, otherwise it just returns the path with no
transformation (i.e. the path to the symlink instead of the
realpath)
- this is not documented _anywhere_ and we were hitting this when
calling `getEmitOutput`, before even using `moduleNameResolver`
- so I just tried implementing it... and it worked!
- notably, the other Rollup TS plugins don't implement this either???
- not sure how they don't error on this??
- note that I added a `!` as `realpath` doesn't have to be implemented
on `ts.sys`... but it is in the default implementation (see comment)
- I originally had a ternary with `fs.realpathSync` if it didn't exist
but that is literally what the default implementation uses
- can add this back in the future if it becomes an issue
* fix realpath test on windows
- 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
* 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?
- _.endsWith -> String.endsWith
- _.concat -> Array.concat
- _.each -> Array.forEach
- _.filter -> Array.filter
- _.map -> Array.map
- _.some -> Array.some
- _.has -> `key in Object`
- _.defaults -> Object.assign
- _.get -> `?.` and `??` (optional chaining and nullish coalescing)
- refactor: replace fairly complicated `expandIncludeWithDirs` func to
just use a few simple `forEach`s
- not as FP anymore, more imperative, but much simpler to read IMO
- refactor: add a `getDiagnostics` helper to DRY up some code
- also aids readability IMO
- a few places are still using lodash, but this paves the way toward
removing it or replacing it with much smaller individual deps
- _.compact still used because Array.filter heavily complicates the
type-checking currently
- _.isFunction still used because while it's a one-liner natively,
need to import a function in several places
- also the package `lodash.isFunction` is lodash v3 and quite
different from the v4 implementation, so couldn't replace with it
unfortunately
- _.merge is a deep merge, so there's no native version of this
- but we may remove deep merges entirely in the future (as tsconfig
doesn't quite perform a deep merge), or could replace this with a
smaller `lodash.merge` package or similar
- see also https://github.com/you-dont-need/You-Dont-Need-Lodash-Underscore
- rename the job a bit to match, and shrink the name a bit for
conciseness as it didn't always fit on screen in the list of checks
- this could be done in a different job as it's not necessarily required
for linting to run through the entire matrix, but this is simpler for
now as we don't have a further need for separate jobs
- and no need to re-run install etc, and the tests are fast anyway
currently, so no perf need there either
- 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
- at least as of TS 2.1: https://www.typescriptlang.org/docs/handbook/utility-types.html#partialtype
- we have a peerDep on TS >=2.4, so should definitely be compatible
- and TS is on ~4.6 at this point, so that's _really_ old
- remove the file and the declarations and declaration maps
- don't rebuild as that's usually done as a separate commit
* 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>
* docs: split off a CONTRIBUTING.md from the README
- condenses the README a bit and uses the standard CONTRIBUTING.md file
- it's a file I often look for when filing an issue or creating a PR
- leaves the section still in the README so that users (esp. on NPM)
will know to go there if they're not aware of that convention
- GitHub also now tells users to read the CONTRIBUTING.md of a repo when
filing issues or creating PRs, so hopefully this helps point more
users in the right direction as well
* docs: improve formatting, grammar, and links in CONTRIBUTING.md
- slightly change reference to GH Issue Tracker
- use oxford commas everywhere for clarity
- missing "the" in a few places
- more minor grammatical fixes (missing space, semicolon vs. comma, etc)
- fix: "npm_modules" -> "`node_modules`"
- fix: "npm lint" -> "npm run lint", "npm build" -> "npm run build",
"npm build-self" -> "npm run build-self"
- short-hand works in Yarn and for some pre-defined Node scripts, like
`start` and `test`, but the rest need `run`
- "typescript" -> "TS" (prefer proper "TypeScript" or just "TS")
- use backticks monospace/code formatting where appropriate
- link to GitHub's official docs on forking and making PRs
- also use the word "standard" instead of "normal" as it's more
inclusive and reflective that this is a convention/standard
- link to editorconfig site
- link directly to `.editorconfig` with a relative link as well
- reword portion about PR checks as they do run `build` and `build-self`
nowadays (not sure how old this text is)
- use an ordered list (instead of unordered) for the testing process
as this is meant to be done in order
- this was introduced in v4.1.0 of @rollup/pluginutils:
https://github.com/rollup/plugins/blob/master/packages/pluginutils/CHANGELOG.md#v410
- this is the same as the code in `normalize.ts` but it uses constants
from Node and is used by multiple Rollup plugins, so just helps with
standardization
- also less code and types to ship in the bundle!
- removed the dist files for `normalize` as well, but didn't do a build
in this commit as those are usually done in separate commits
- "latest 2.x" is no longer accurate and quite outdated (we're on 4.x
now), so just say it's the peerDep instead
- @rollup/plugin-typescript calls the default a peerDep too:
https://github.com/rollup/plugins/tree/master/packages/typescript#typescript
- be more specific that you can pass in a different version or fork of
TS (like `ttypescript`) through this option
- follow-up to 8ec49c78f523687deaf6816bc2ea320f16e325c7
- auto-trim some whitespace-only lines
- my editor does this automatically, and most of the README has
trimmed whitespace anyway, so this keeps it consistent
- `rollup-plugin-` -> `@rollup/plugin-`
- just less confusion for newer users who may not be aware of the
previous naming. and up-to-date / current naming anyway
- clarify grammar in `@rollup/plugin-babel` to be a bit more specific
- remove part about what "it claims", no need for negative tone
- add a link to the Babel plugin's docs for its default extensions
- comment out ellipsis in code samples (closer to valid code)
- it had a deprecation warning on install stating that types were
built-in now
- confirmed that my editor picks up the types fine without and that
builds still work
- auto-alphabetize deps
- update package-lock.json version to match package.json version
- NPM did both of these automatically when I ran `npm install`
- fix: missing `#` in `outDir` reference ("83")
- use `org/repo#num` consistently
- previously was `Microsoft/TypeScript/issues/num` or
`rollup/issues/num` inconsistently
- this is how GitHub displays references to other repos in comments as
well, so consistent with the rest of GH too
- links haven't changed, just the text of the them
- auto-trim some whitespace-only lines
- my editor does this automatically, and most of the README has
trimmed whitespace anyway, so this keeps it consistent
- previously it just linked to the image itself; better to link to the
source of the stats which is NPM
- duplicates the other badge's link, but nbd, better than linking to
an image imo
- it's a deep merge that merges them by index
- the rest of the docs are accurate, just this one mention was incorrect
- eventually should move to shallow merge/replace arrays to better
reflect how `tsconfig` `extends` works, but just fix the docs for now
- I originally made the spoilers and while they work (and I would say
improve readability a good bit), there's also some issues with them
- I've used them a ton more now so know how to workaround most
of the issues with them now
- fix: use HTML `code` tag inside of `summary` tag, can't use backticks
- was mixing MD and HTML before, and this doesn't always work and
didn't work on GitHub, they just had backticks
- fix: don't duplicate the file name in the heading, just make the
`summary` have a heading inside it instead
- use an `h4` same as the `####` that it was before
- feat: add syntax highlighting by adding code blocks for each code
snippet
- js for rollup.config.js, json5 for tsconfig (it has comments,
trailing commas, etc (actually a custom parser, but json5 is close
enough)), json for package.json, and text for verbose logs
- also, a lot of people sometimes just paste the code with no code
block and it formats terribly, so this should help defer that
(as well as any potential issues that can crop up with unindented
blocks)
- the `envinfo` text code block seems to be working well, so
hopefully this will improve issues too
