-
Notifications
You must be signed in to change notification settings - Fork 71
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
test: dogfood rpt2 as configPlugin
w/ rollup.config.ts
#460
base: master
Are you sure you want to change the base?
Conversation
- use rpt2 as a `configPlugin` within our own build process, as well as to build itself - doubly dogfood - we don't have any tests for `configPlugin`s, and it's a Rollup CLI option, so this is one of the few ways to test it - create a helper `_build-self` script so we don't need to type/repeat as much (or have as much potential for mistakes) NOTE: this commit fails to build, need to make some more adjustments to the base config to get it to work in TS - will do that in the next commits so that it's easier to read/differentiate/review
- replace the unmaintained `rollup-plugin-re` w/ `@rollup/plugin-replace` - `rollup-plugin-re` hasn't had a commit in ~5 years and doesn't have typings (natively or in DT) - also import the plugins in the same order that they're used - `require("package.json")` -> `import` - and add `resolveJsonModule` to `configPlugin` `tsconfigOverride` to handle this - only used by the `configPlugin`, not everything else, so not added to `tsconfig.json` - fix the usage of the `resolve` plugin as it didn't type-check/had outdated usage
- need to use `createRequire` in ESM mode - deps: bump `@types/node` to support the types for that as well - this is a BREAKING CHANGE as it requires Node 14+ - fix `fs-extra` error due to it being CJS - add `build-self/*` to `prebuild` clean - not to `build-self` script, as the `build-self/` directory has to exist for that - output as `build-self/index.mjs` instead of `build-self/dist/rollup-plugin-typescript2.es.js` - so that types work out of the box -- due to `index.d.ts` - and so that Rollup correctly loads it as ESM and not CJS
As this is a breaking change due to use of |
- set `engines.node` in `package.json` to show this as well
Tests are failing due to use of |
I'm also getting an error on Windows trying one of the
|
Dang thought we were past all Windows pathing issues 😅 Looks like something is expanding/converting the path in a way Rollup doesn't like |
Summary
Dogfood rpt2 more by using it as a
configPlugin
on our own internalrollup.config.ts
Details
Motivation
use rpt2 as a
configPlugin
within our own build process, as well as to build itselfwe don't have any tests for
configPlugin
s, and it's a Rollup CLI option, so this is one of the few ways to test it_build-self
script so we don't need to type/repeat as much (or have as much potential for mistakes)0.34.0
not handling JSON import inrollup.config.ts
#426TS Fixes
replace the unmaintained
rollup-plugin-re
w/@rollup/plugin-replace
rollup-plugin-re
hasn't had a commit in ~5 years and doesn't have typings (natively or in DT)fix the usage of the
resolve
plugin as it didn't type-check/had outdated usageESM Fixes
need to use
createRequire
in ESM mode@types/node
to support the types for that as wellfix
fs-extra
error due to it being CJSrequire("package.json")
->import
resolveJsonModule
toconfigPlugin
tsconfigOverride
to handle thisconfigPlugin
, not everything else, so not added totsconfig.json
output as
build-self/index.mjs
instead ofbuild-self/dist/rollup-plugin-typescript2.es.js
index.d.ts
Misc
#456 was split out of this as it took me quite a while to get builds to pass