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

Update npm packages #326

Merged
merged 36 commits into from
Jan 17, 2024
Merged

Update npm packages #326

merged 36 commits into from
Jan 17, 2024

Conversation

mob8607
Copy link
Collaborator

@mob8607 mob8607 commented Sep 11, 2023

Pull request

Updates outdated npm packages.

Currently not updated:

  • Typescript from 4 to 5
  • storybook from 6.5 to 7.6

Browser testing

Checklist

  • I merged the current development branch (before testing)
  • Added JSDoc and styleguide demo
  • Tested all links in project relevant browsers
  • Tested all links in different screen sizes
  • Did run automated tests and linters
  • Did assign ticket
  • Double checked target branch

Review/Test checklist

  • Did review code and documentation
  • Tested all links in project relevant browsers
  • Tested all links in different screen sizes
  • Did check accessibility (Wave, only errors)
  • Re-assign ticket to developer

@patric-eberle patric-eberle changed the base branch from master to develop October 19, 2023 08:58
Copy link
Member

@patric-eberle patric-eberle left a comment

Choose a reason for hiding this comment

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

Browser review

  • A new major storybook version is available.
grafik
  • There is an error about a storybook add-on when running storybook:
grafik
  • There are two errors about static vs. dynamic imports when building. I think the one about api can be resolved by always do a static import. I think the other one can not be resolved.
grafik

src/stories/components/c-modal.stories.js Outdated Show resolved Hide resolved
tests/unit/specs/plugins/vue-bem/testingEntitys.ts Outdated Show resolved Hide resolved
tests/unit/specs/plugins/vue-bem/testingEntitys.ts Outdated Show resolved Hide resolved
tests/unit/specs/plugins/vue-bem/testingEntitys.ts Outdated Show resolved Hide resolved
tsconfig.json Outdated Show resolved Hide resolved
@mob8607
Copy link
Collaborator Author

mob8607 commented Oct 23, 2023

  • There are two errors about static vs. dynamic imports when building. I think the one about api can be resolved by always do a static import. I think the other one can not be resolved.

I could not fix this without introducing another problem either in storybook or in the tests. I will for now leave it as it is. But we should investigate this a bit more, because to me it looks like we have a setup issue here.

@patric-eberle
Copy link
Member

@mob8607 this PR is still open. Do you have the possibility to check your questions with someone else? Not sure, when I will be ready to check your questions with you...

@mob8607
Copy link
Collaborator Author

mob8607 commented Jan 2, 2024

@patric-eberle @benjamin-joham
I updated the dependencies again. This time the stylelint version is updated (That's why i needed the commit for the valantic-styleguide dependency). This means that we now should integrate prettier in this release, because some of our rules are now dropped from stylelint.

@patric-eberle
Copy link
Member

  • There are two errors about static vs. dynamic imports when building. I think the one about api can be resolved by always do a static import. I think the other one can not be resolved.

I could not fix this without introducing another problem either in storybook or in the tests. I will for now leave it as it is. But we should investigate this a bit more, because to me it looks like we have a setup issue here.

@mob8607 can you please create a ticket about what needs to be checked? I currently can't remember the whole context of this while you might do ;)

Copy link
Member

@patric-eberle patric-eberle left a comment

Choose a reason for hiding this comment

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

Added inputs about some package versions. Please double check.

  • There is an endless list of warnings when running npm run test. I think this could be caused by the invalid vite version: [eslint-plugin-import:resolver:vite] Cannot use import statement outside a module
  • npm run storybook shows the following warning. Can we resolve this?
grafik

"vite": "~4.4.9",
"svgo": "~3.2.0",
"typescript": "~5.3.3",
"vite": "~4.4.12",
Copy link
Member

Choose a reason for hiding this comment

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

todo: according to the release information, @vitejs/plugin-vue in version 5 is no longer compatible with vite 4.x: https://github.com/vitejs/vite-plugin-vue/blob/main/packages/plugin-vue/CHANGELOG.md#500-2023-12-25

package.json Outdated
"sass": "~1.69.6",
"storybook": "~7.6.7",
"stylelint": "~16.1.0",
"stylelint-config-valantic": "github:valantic/stylelint-config-valantic#48b8f70490dc0b4bae69518d7f9efc2e762eda55",
Copy link
Member

Choose a reason for hiding this comment

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

todo: please make sure we create a release of sylelint-config-valantic before this is merged.

@@ -119,12 +119,11 @@
* For browser support @see https://caniuse.com/?search=load
*/
loading: {
type: String,
type: String as PropType<'lazy' | 'eager'>,
Copy link
Member

Choose a reason for hiding this comment

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

Nice change 👍

@patric-eberle
Copy link
Member

@mob8607 can you please resolve the merge conflicts before I check this again. THX.

…re/npm-updates

 Conflicts:
	package-lock.json
	package.json
	src/styleguide/components/s-api-mock-test.vue
	src/styleguide/mock-data/data-object/image.ts
	tests/unit/specs/plugins/vue-bem/testingEntitys.ts
	tsconfig.json
…re/npm-updates

 Conflicts:
	src/styleguide/api/handler.js
@mob8607
Copy link
Collaborator Author

mob8607 commented Jan 16, 2024

@patric-eberle It should now be up to date and ready

@mob8607 mob8607 mentioned this pull request Jan 16, 2024
@patric-eberle
Copy link
Member

@mob8607 changes look good to me. Thank you for your patience. I was not able to force the lint-staged issue on my machine. Can you also document in this ticket, why you had to go back to vite 4? #350

THX.

@patric-eberle patric-eberle merged commit ccda2b3 into develop Jan 17, 2024
@patric-eberle patric-eberle deleted the feature/npm-updates branch January 17, 2024 12:27
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.

4 participants