-
-
Notifications
You must be signed in to change notification settings - Fork 58
fix(valid-compile): use compiler options if provided #1373
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
base: main
Are you sure you want to change the base?
Conversation
🦋 Changeset detectedLatest commit: e6122d9 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
ef91ca5
to
e370871
Compare
Try the Instant Preview in Online PlaygroundInstall the Instant Preview to Your Local
Published Instant Preview Packages:
|
e370871
to
e6122d9
Compare
} { | ||
try { | ||
const result = compiler.compile(code, { | ||
...context.sourceCode.parserServices.svelteParseContext?.svelteConfig?.compilerOptions, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To avoid unintended behavior, can you please pass only the necessary options?
Since generate: false
is different from the compiler used by the user, I think it is highly likely that the user's defined compilerOptions
cannot be used as is.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you explain this in a bit more detail?
If generate: false
is specified, the Svelte compiler just skips the transform phase, so I don’t think it would be a problem to overwrite the user’s settings for generate
. Also, options that are only used in the transform phase would simply go unused, so I don’t think they would have any side effects.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Compiler options have many properties, and I don't think we understand how they affect linting. Also, I don't think users set compiler options with linting in mind.
Therefore, I think it's better to avoid passing them directly.
I think it's better to pass only the necessary properties that we understand.
https://svelte.dev/docs/svelte/svelte-compiler#CompileOptions
https://svelte.dev/docs/svelte/svelte-compiler#ModuleCompileOptions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we understand how they affect linting
I believe I understand your concern. I have no objection to making the fix.
However, in this particular case, do we really need to worry about the affect? Wouldn’t it be fine to simply compile exactly as the user specified and then display whatever warnings are generated to the user? If something strange happens, wouldn’t that just mean the user’s configuration was wrong?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The reason I'm concerned may be because I don't know the internals of the compiler, so I don't have the information to not worry 😓
close: #1242