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 preprocessors tree by wrapping with moduleName #1205

Merged
merged 6 commits into from
Jun 2, 2022

Conversation

wondersloth
Copy link
Contributor

@wondersloth wondersloth commented May 19, 2022

This PR addresses the following issues:

Replicated the issue here:
https://github.com/wondersloth/demo-embroider-preprocessors-bug

In a classic build the tree passed to the preprocessor is scoped with the moduleName.

In embroider, th app or addon tree is not scoped with the name/moduleName

screenshot

This creates backcompat issues with addons like ember-css-modules that uses the file path to generate hash for some values. Initial attempt fixing from an addon.

  • Validated fix via yarn link @embroider/compat in demo-embroider-preprocessors-bug
  • Fix v1-app.ts to scope/wrap with moduleName when passing to preprocessors.
  • Fix tests so that preprocessors apply transform to my-addon.

@wondersloth wondersloth changed the title Tree passed to preprocessors for app and addon is not scoped with moduleName WIP Tree passed to preprocessors for app and addon is not scoped with moduleName May 19, 2022
@wondersloth wondersloth changed the title WIP Tree passed to preprocessors for app and addon is not scoped with moduleName Tree passed to preprocessors for app and addon is not scoped with moduleName May 19, 2022
@wondersloth wondersloth changed the title Tree passed to preprocessors for app and addon is not scoped with moduleName Fix preprocessors tree by wrapping with moduleName May 19, 2022
@SergeAstapov SergeAstapov added the bug Something isn't working label May 19, 2022
@wondersloth
Copy link
Contributor Author

wondersloth commented May 19, 2022

@ef4 I've got my tests for the preprocessors to pass. FWIW the entire test suite passes for @embroider/compat on my local.

Next would be to validate this more broadly.

@wondersloth
Copy link
Contributor Author

wondersloth commented May 20, 2022

In my integration testing with the example apps in ember-css-modules, I found another bug.
I need to:

  • Move the unwrapping of the moduleName from the files, because the template ast transforms already get the moduleName in the path.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants