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

(fix): change plugin order to make styled-components/macro work #644

Merged
merged 1 commit into from
Sep 17, 2020

Conversation

agilgur5
Copy link
Collaborator

  • if babel-plugin-macros is first, macros get applied first

    • babel-plugin-styled-components says it should be placed first, but
      unfortunately all user plugins are added after TSDX plugins
      • unknown how changing this ordering could impact lots of code out
        there, but could be very breaking.
        • moving to a preset instead would mean this is more in user
          control
    • but we can change it so macros are added first and so one can use
      styled-components/macro instead as a workaround
  • based on babel-plugin-styled-component's code, docs, and output in
    detail, I think its ordering conflict may be with
    babel-plugin-annotate-pure-calls, which so happens to be the first
    plugin in babelPluginTsdx

    • maybe this should be last given that other plugins can change
      functions etc

(test): update styled-component template tag test to reflect the
slightly different tag due to the usage of the macro

(fix/test): comment removal should use toBeFalsy, not toBeTruthy,
since it's removed

  • this was a bug I introduced when adding the grep helper; just added
    it the same everywhere but this was the one place that was testing
    to get an error code
    • fix the comment so it doesn't say error code anymore either
    • since this test was skipped, I didn't pick up that it was wrong
      until it ran now

Workaround for #543

Should go out in a minor since it's possible changing the order of this one plugin could break some things, but highly unlikely

@agilgur5 agilgur5 added the version: minor Increment the minor version when merged label Mar 27, 2020
@agilgur5 agilgur5 added this to the v0.14.0 milestone Mar 27, 2020
@zebapy

This comment has been minimized.

@agilgur5

This comment has been minimized.

@agilgur5 agilgur5 added the scope: integration Related to an integration, not necessarily to core (but could influence core) label Sep 17, 2020
@vercel

This comment has been minimized.

Copy link
Collaborator Author

@agilgur5 agilgur5 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One nit, otherwise seems good to go if all tests pass

test/integration/tsdx-build-withBabel.test.ts Outdated Show resolved Hide resolved
- if babel-plugin-macros is first, macros get applied first
  - babel-plugin-styled-components says it should be placed first, but
    unfortunately all user plugins are added after TSDX plugins
    - unknown how changing this ordering could impact lots of code out
      there, but could be very breaking.
      - moving to a preset instead would mean this is more in user
        control
  - but we can change it so macros are added first and so one can use
    styled-components/macro instead as a workaround

- based on babel-plugin-styled-component's code, docs, and output in
  detail, I think its ordering conflict may be with
  babel-plugin-annotate-pure-calls, which so happens to be the first
  plugin in babelPluginTsdx
  - maybe this should be last given that other plugins can change
    functions etc

(test): update styled-component template tag test to reflect the
slightly different tag due to the usage of the macro

(fix/test): comment removal should use toBeFalsy, not toBeTruthy,
since it's removed
- this was a bug I introduced when adding the grep helper; just added
  it the same everywhere but this was the one place that was testing
  to get an error code
  - fix the comment so it doesn't say error code anymore either
  - since this test was skipped, I didn't pick up that it was wrong
    until it ran now
@agilgur5 agilgur5 merged commit 6d7257c into jaredpalmer:master Sep 17, 2020
paul-vd pushed a commit to EezyQuote/tsdx that referenced this pull request Dec 1, 2020
…dpalmer#644)

- if babel-plugin-macros is first, macros get applied first
  - babel-plugin-styled-components says it should be placed first, but
    unfortunately all user plugins are added after TSDX plugins
    - unknown how changing this ordering could impact lots of code out
      there, but could be very breaking.
      - moving to a preset instead would mean this is more in user
        control
  - but we can change it so macros are added first and so one can use
    styled-components/macro instead as a workaround

- based on babel-plugin-styled-component's code, docs, and output in
  detail, I think its ordering conflict may be with
  babel-plugin-annotate-pure-calls, which so happens to be the first
  plugin in babelPluginTsdx
  - maybe this should be last given that other plugins can change
    functions etc

(test): update styled-component template tag test to reflect the
slightly different tag due to the usage of the macro

(fix/test): comment removal should use toBeFalsy, not toBeTruthy,
since it's removed
- this was a bug I introduced when adding the grep helper; just added
  it the same everywhere but this was the one place that was testing
  to get an error code
  - fix the comment so it doesn't say error code anymore either
  - since this test was skipped, I didn't pick up that it was wrong
    until it ran now
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
scope: integration Related to an integration, not necessarily to core (but could influence core) version: minor Increment the minor version when merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants