Skip to content
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

clean: remove ConsoleContext entirely by using buildStart #414

Merged
merged 1 commit into from
Aug 29, 2022

Conversation

agilgur5
Copy link
Collaborator

Summary

Remove ConsoleContext (and the need for it) by using the buildStart hook instead of the options hook

Details

  • 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
        • I'm not sure which is the better one to hash, but I left it as is so that the hash remains the same
      • 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 throwing errors in parse-tsconfig as it exists now

- 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. ff88951, 0628482
  - 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
@agilgur5 agilgur5 added the kind: internal Changes only affect the internals, and _not_ the public API or external-facing docs label Aug 25, 2022
@agilgur5 agilgur5 requested a review from ezolenko August 25, 2022 05:40
@ezolenko ezolenko merged commit 1e71d50 into ezolenko:master Aug 29, 2022
@agilgur5 agilgur5 deleted the clean-remove-console-context branch July 2, 2023 21:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: internal Changes only affect the internals, and _not_ the public API or external-facing docs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants