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

remove assetsDir backwards compatibility #3341

Merged
merged 5 commits into from
Apr 21, 2022

Conversation

alicewriteswrongs
Copy link
Contributor

@alicewriteswrongs alicewriteswrongs commented Apr 20, 2022

Pull request checklist

Please check if your PR fulfills the following requirements:

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been reviewed and added / updated if needed (for bug fixes / features)
  • Build (npm run build) was run locally and any changes were pushed
  • Unit tests (npm test) were run locally and passed
  • E2E Tests (npm run test.karma.prod) were run locally and passed
  • Prettier (npm run prettier) was run locally and passed

Pull request type

Please check the type of change your PR introduces:

  • Bugfix
  • Feature
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • Documentation content changes
  • Other (please describe): removing backwards compatibility for a deprecated parameter

What is the current behavior?

GitHub Issue Number: N/A

Right now we have backwards compatibility for the assetsDir option by converting it under the hood from string to string[] and setting it on assetsDirs. This removes that, since the param has been deprecated for quite some time now.

What is the new behavior?

It will no longer allow the backwards compatibility.

Does this introduce a breaking change?

  • Yes
  • No

Users will need to remove any assetsDir props passed in to the @Component decorator, and migrate to assetsDirs instead.

Testing

I ran the unit tests and also installed this into a test component repo to verify that things are working alright

Other information

The `assetsDir` option for the `@Component` decorator was deprecated
some time ago in favor of the `assetsDirs` option, but we have retained
backwards-compatibility with the old option nonetheless. This commit
removes that backwards compatibility.

STENCIL-410: Remove Backwards Compatibility for assetDir Field on @component

BREAKING CHANGE: setting `assetsDir` on a component will no longer work.
Users should migrate to `assetsDirs` instead.
@alicewriteswrongs alicewriteswrongs requested a review from a team April 20, 2022 20:47
Comment on lines 43 to 47
if (isString((componentOptions as any).assetsDirs)) {
const error = buildError(diagnostics);
error.messageText = `@Component option "assetsDir" is no longer supported. "assetsDirs" should be used instead.`
augmentDiagnosticWithNode(error, componentDecorator)
}
Copy link
Member

Choose a reason for hiding this comment

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

We may be able to delete this conditional in its entirety. ATM, assetsDirs is typed as a string[]:

  /**
   * Array of relative links to folders of assets required by the component.
   */
  assetsDirs?: string[];

Screen Shot 2022-04-20 at 7 27 44 PM

I think if folks are still using it as a string literal, we can lean on the TS type checker to yell at them. Thoughts?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I meant to leave a review comment on this but forgot - my thought initially was to just remove the whole conditional, but I wasn't sure if we wanted to try to have some sort of last-ditch error throwing deprecation warning. But I'm for removing it entirely if you think that makes sense :)

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, I think it's acceptable at this point from a usability standpoint. assetsDir: string was deprecated in v2.0.0, so I'd argue folks have definitely had enough time to migrate to assetsDir: string[]

@rwaskiewicz rwaskiewicz merged commit eb61f89 into v3.0.0-dev Apr 21, 2022
@rwaskiewicz rwaskiewicz deleted the ap/remove-asset-dir-compat branch April 21, 2022 14:51
alicewriteswrongs added a commit that referenced this pull request Apr 25, 2022
In #3341 we removed backwards compatibility for the deprecated
`assetsDir` component decorator prop. This updates the BREAKING_CHANGES
document to reflect that change.

STENCIL-410: Remove Backwards Compatibility for assetDir Field on @component
rwaskiewicz pushed a commit that referenced this pull request Apr 25, 2022
…pat (#3343)

In #3341 we removed backwards compatibility for the deprecated
`assetsDir` component decorator prop. This updates the BREAKING_CHANGES
document to reflect that change.

STENCIL-410: Remove Backwards Compatibility for assetDir Field on @component
rwaskiewicz pushed a commit that referenced this pull request Oct 3, 2022
the `assetsDir` option for the `@Component` decorator was deprecated
some time ago in favor of the `assetsDirs` option, but we have retained
backwards-compatibility with the old option nonetheless. this commit
removes that backwards compatibility.

STENCIL-410: Remove Backwards Compatibility for assetDir Field on @component

BREAKING CHANGE: setting `assetsDir` on a component will no longer work.
Users should migrate to `assetsDirs` instead.
rwaskiewicz pushed a commit that referenced this pull request Oct 3, 2022
…pat (#3343)

In #3341 we removed backwards compatibility for the deprecated
`assetsDir` component decorator prop. This updates the BREAKING_CHANGES
document to reflect that change.

STENCIL-410: Remove Backwards Compatibility for assetDir Field on @component
rwaskiewicz pushed a commit that referenced this pull request Dec 2, 2022
the `assetsDir` option for the `@Component` decorator was deprecated
some time ago in favor of the `assetsDirs` option, but we have retained
backwards-compatibility with the old option nonetheless. this commit
removes that backwards compatibility.

STENCIL-410: Remove Backwards Compatibility for assetDir Field on @component

BREAKING CHANGE: setting `assetsDir` on a component will no longer work.
Users should migrate to `assetsDirs` instead.
rwaskiewicz pushed a commit that referenced this pull request Dec 2, 2022
…pat (#3343)

In #3341 we removed backwards compatibility for the deprecated
`assetsDir` component decorator prop. This updates the BREAKING_CHANGES
document to reflect that change.

STENCIL-410: Remove Backwards Compatibility for assetDir Field on @component
rwaskiewicz pushed a commit that referenced this pull request Dec 5, 2022
the `assetsDir` option for the `@Component` decorator was deprecated
some time ago in favor of the `assetsDirs` option, but we have retained
backwards-compatibility with the old option nonetheless. this commit
removes that backwards compatibility.

STENCIL-410: Remove Backwards Compatibility for assetDir Field on @component

BREAKING CHANGE: setting `assetsDir` on a component will no longer work.
Users should migrate to `assetsDirs` instead.
rwaskiewicz pushed a commit that referenced this pull request Dec 5, 2022
…pat (#3343)

In #3341 we removed backwards compatibility for the deprecated
`assetsDir` component decorator prop. This updates the BREAKING_CHANGES
document to reflect that change.

STENCIL-410: Remove Backwards Compatibility for assetDir Field on @component
rwaskiewicz pushed a commit that referenced this pull request Jan 23, 2023
the `assetsDir` option for the `@Component` decorator was deprecated
some time ago in favor of the `assetsDirs` option, but we have retained
backwards-compatibility with the old option nonetheless. this commit
removes that backwards compatibility.

STENCIL-410: Remove Backwards Compatibility for assetDir Field on @component

BREAKING CHANGE: setting `assetsDir` on a component will no longer work.
Users should migrate to `assetsDirs` instead.
rwaskiewicz pushed a commit that referenced this pull request Jan 23, 2023
…pat (#3343)

In #3341 we removed backwards compatibility for the deprecated
`assetsDir` component decorator prop. This updates the BREAKING_CHANGES
document to reflect that change.

STENCIL-410: Remove Backwards Compatibility for assetDir Field on @component
rwaskiewicz pushed a commit that referenced this pull request Jan 24, 2023
the `assetsDir` option for the `@Component` decorator was deprecated
some time ago in favor of the `assetsDirs` option, but we have retained
backwards-compatibility with the old option nonetheless. this commit
removes that backwards compatibility.

STENCIL-410: Remove Backwards Compatibility for assetDir Field on @component

BREAKING CHANGE: setting `assetsDir` on a component will no longer work.
Users should migrate to `assetsDirs` instead.
rwaskiewicz pushed a commit that referenced this pull request Jan 24, 2023
…pat (#3343)

In #3341 we removed backwards compatibility for the deprecated
`assetsDir` component decorator prop. This updates the BREAKING_CHANGES
document to reflect that change.

STENCIL-410: Remove Backwards Compatibility for assetDir Field on @component
rwaskiewicz pushed a commit that referenced this pull request Jan 24, 2023
the `assetsDir` option for the `@Component` decorator was deprecated
some time ago in favor of the `assetsDirs` option, but we have retained
backwards-compatibility with the old option nonetheless. this commit
removes that backwards compatibility.

STENCIL-410: Remove Backwards Compatibility for assetDir Field on @component

BREAKING CHANGE: setting `assetsDir` on a component will no longer work.
Users should migrate to `assetsDirs` instead.
rwaskiewicz pushed a commit that referenced this pull request Jan 24, 2023
…pat (#3343)

In #3341 we removed backwards compatibility for the deprecated
`assetsDir` component decorator prop. This updates the BREAKING_CHANGES
document to reflect that change.

STENCIL-410: Remove Backwards Compatibility for assetDir Field on @component
rwaskiewicz pushed a commit that referenced this pull request Jan 24, 2023
the `assetsDir` option for the `@Component` decorator was deprecated
some time ago in favor of the `assetsDirs` option, but we have retained
backwards-compatibility with the old option nonetheless. this commit
removes that backwards compatibility.

STENCIL-410: Remove Backwards Compatibility for assetDir Field on @component

BREAKING CHANGE: setting `assetsDir` on a component will no longer work.
Users should migrate to `assetsDirs` instead.
rwaskiewicz pushed a commit that referenced this pull request Jan 24, 2023
…pat (#3343)

In #3341 we removed backwards compatibility for the deprecated
`assetsDir` component decorator prop. This updates the BREAKING_CHANGES
document to reflect that change.

STENCIL-410: Remove Backwards Compatibility for assetDir Field on @component
rwaskiewicz pushed a commit that referenced this pull request Jan 24, 2023
the `assetsDir` option for the `@Component` decorator was deprecated
some time ago in favor of the `assetsDirs` option, but we have retained
backwards-compatibility with the old option nonetheless. this commit
removes that backwards compatibility.

STENCIL-410: Remove Backwards Compatibility for assetDir Field on @component

BREAKING CHANGE: setting `assetsDir` on a component will no longer work.
Users should migrate to `assetsDirs` instead.
rwaskiewicz pushed a commit that referenced this pull request Jan 24, 2023
…pat (#3343)

In #3341 we removed backwards compatibility for the deprecated
`assetsDir` component decorator prop. This updates the BREAKING_CHANGES
document to reflect that change.

STENCIL-410: Remove Backwards Compatibility for assetDir Field on @component
rwaskiewicz pushed a commit that referenced this pull request Jan 25, 2023
the `assetsDir` option for the `@Component` decorator was deprecated
some time ago in favor of the `assetsDirs` option, but we have retained
backwards-compatibility with the old option nonetheless. this commit
removes that backwards compatibility.

STENCIL-410: Remove Backwards Compatibility for assetDir Field on @component

BREAKING CHANGE: setting `assetsDir` on a component will no longer work.
Users should migrate to `assetsDirs` instead.
rwaskiewicz pushed a commit that referenced this pull request Jan 25, 2023
…pat (#3343)

In #3341 we removed backwards compatibility for the deprecated
`assetsDir` component decorator prop. This updates the BREAKING_CHANGES
document to reflect that change.

STENCIL-410: Remove Backwards Compatibility for assetDir Field on @component
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants