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(dynamic-import-vars): normalize paths at runtime #1794

Conversation

bearfriend
Copy link
Contributor

Rollup Plugin Name: dynamic-import-vars

This PR contains:

  • bugfix
  • feature
  • refactor
  • documentation
  • other

Are tests included?

  • yes (bugfixes and features will not be merged without tests)
  • no

Breaking Changes?

  • yes (breaking changes will not be merged unless absolutely necessary)
  • no

If yes, then include "BREAKING CHANGES:" in the first commit message body, followed by a description of what is breaking.

List any relevant issue numbers: #1791

Description

Fixes #1791 by normalizing paths at runtime to match the glob results. I decided again using a port of node's posix.normalize to both avoid forcing dependencies on consumers and also because the existing restrictions of the plugin mean it was (relatively) easy to transform the paths into how we need them.

@@ -234,25 +297,3 @@ Generated by [AVA](https://avajs.dev).
export { importModule };␊
`

## no files in dir
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do I just add this back manually?

@bearfriend bearfriend changed the title [plugin-dynamic-import-vars] normalize paths at runtime fix(plugin-dynamic-import-vars) normalize paths at runtime Oct 18, 2024
@bearfriend bearfriend changed the title fix(plugin-dynamic-import-vars) normalize paths at runtime fix(plugin-dynamic-import-vars): normalize paths at runtime Oct 18, 2024
@bearfriend bearfriend force-pushed the dgleckler/dynamic-import-vars-normalize-paths branch 2 times, most recently from e6359f3 to 16b1fd7 Compare October 18, 2024 21:56
@shellscape shellscape changed the title fix(plugin-dynamic-import-vars): normalize paths at runtime fix(dynamic-import-vars): normalize paths at runtime Oct 21, 2024
@shellscape
Copy link
Collaborator

Really appreciate the work on this. However, there are still some failing tests and this ends up injecting more code into a bundle on the resulting end. I thought on this for a while this weekend, and we're going to revert this plugin back to the previous version's state, since the only benefit to the change was fewer dependencies, and the juice is just not worth the squeeze.

@shellscape shellscape closed this Oct 21, 2024
@bearfriend bearfriend deleted the dgleckler/dynamic-import-vars-normalize-paths branch October 21, 2024 12:42
@bearfriend
Copy link
Contributor Author

bearfriend commented Oct 21, 2024

I think the tests got into an odd state from some force pushes, and should have been passing otherwise. Not sure, though.

Regardless, the changes were weirder than I thought they'd be. I support reverting.

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.

[plugin-dynamic-import-vars] non-normalized paths no longer work
2 participants