Skip to content
This repository has been archived by the owner on Jan 18, 2022. It is now read-only.

fix: generate compiler per component to pass information about scopeId #337

Merged
merged 2 commits into from May 11, 2020

Conversation

przemkow
Copy link
Contributor

@przemkow przemkow commented Apr 11, 2020

Fixes #321

Changes proposed in this pull request:

  • Generate compiler instance per component so component scopeId can be passed as a variable to generate correctly optimised SSR code.

I tried to add tests for that but it seems that currently tests enviroment supports only iife build.

/ping @znck

@znck
Copy link
Member

znck commented Apr 14, 2020

It looks good but I need some time to properly review the code (😞 as CI is broken).

@przemkow
Copy link
Contributor Author

yea I noticed that. No worries @znck, take you time :) if you notice that something needs to be changed let me know :)

@hjkcai
Copy link

hjkcai commented May 11, 2020

How is it going here? I have the same problem and I really need this fix. Thanks @znck @przemkow

BTW in the fix code, is it fine to create the compiler multiple times? I am not sure about it. My fix might be simpler:

if (descriptor.template) {
  const hasScoped = descriptor.styles.some(s => !!s.scoped);

  // magic!
  Object.assign(compiler.template.compilerOptions, { scopeId: hasScoped ? scopeId : undefined });

  // ...
}

@przemkow
Copy link
Contributor Author

@hjkcai Thanks for that suggestion! To be honest I was thinking about something similar but I wasn't sure if mutating the compilerOptions will not create any side-effects.
I noticed that vue-loader seems to create a separate instance of templateLoader per file so I tried to implement a similar approach with as small modifications as possible.
If you have some time @znck let us know what do you think about that :) If any modifications are required I will try to update this PR as soon as possible.

@znck
Copy link
Member

znck commented May 11, 2020

Transform function is async and mutation could potentially produce wrong results.

P.S. I’ll merge it today. Apologies for the delay.

@znck znck merged commit 1fec2c5 into vuejs:master May 11, 2020
@hjkcai
Copy link

hjkcai commented May 12, 2020

Good job @znck @przemkow

Thanks a lot

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

No scoped attribute in children elements with scss
3 participants