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.

Sign up for GitHub

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

Jump to bottom

feat: add option to force-start svelte plugin #2185

Merged
merged 1 commit into from
Oct 27, 2023
Merged

feat: add option to force-start svelte plugin #2185

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion packages/typescript-plugin/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,10 @@ Then add it to your `tsconfig.json` or `jsconfig.json`:
"compilerOptions": {
...
"plugins": [{
"name": "typescript-svelte-plugin"
"name": "typescript-svelte-plugin",
// the following options can be set additionally; they are optional; their default values are listed here
"enabled": true, // enables this plugin
"assumeIsSvelteProject": false // if true, skip detection and always assume it's a Svelte project
}]
}
}
Expand Down
5 changes: 4 additions & 1 deletion packages/typescript-plugin/src/config-manager.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,15 @@ const configurationEventName = 'configuration-changed';

export interface Configuration {
enable: boolean;
/** Skip the Svelte detection and assume this is a Svelte project */
assumeIsSvelteProject: boolean;
}

export class ConfigManager {
private emitter = new EventEmitter();
private config: Configuration = {
enable: true
enable: true,
assumeIsSvelteProject: false
Copy link

Choose a reason for hiding this comment

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

@dummdidumm looks great, thanks!

Just to double check, isConfigChanged in this class still only reacts to enable, is that alright?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes that's right. I implemented this way because assumeIsSvelteProject is the very first thing that is checked, and everything else is just skipped else, so no point in reacting to something that can never be reacted to you.

Is that a problem for the Intellij integration because you create the TypeScript service eagerly, and therefore all plugins eagerly, and what you need instead is some kind of "sleep" mode where things are already prepared but not turned on yet in the plugin? I imagine that being tricky, I don't know how TypeScript would react if we lazily patch stuff. Probably the closest to this is the enable flag which turns off all the intellisense-related work when it's false, so it's almost like the sleep mode I described.

Copy link

Choose a reason for hiding this comment

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

On second take your current code is fine. No, we create TypeScript service when the first TS file is opened and we control which commands are sent in what order. We never had to use "configurePlugin" before actually and my initial understanding was wrong, so I was getting undefined at creation time. After a bit of investigation and a bug fix in protocol handling, now it works as expected 👍

Type checks for Svelte compiled classes work without node_modules installed. Zero-effort types are not injected but after npm install everything refreshes properly.

Thanks again!

};

onConfigurationChanged(listener: (config: Configuration) => void) {
Expand Down
5 changes: 4 additions & 1 deletion packages/typescript-plugin/src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,10 @@ function init(modules: { typescript: typeof ts }): ts.server.PluginModule {

function create(info: ts.server.PluginCreateInfo) {
const logger = new Logger(info.project.projectService.logger);
if (!isSvelteProject(info.project.getCompilerOptions())) {
if (
!(info.config as Configuration)?.assumeIsSvelteProject &&
!isSvelteProject(info.project.getCompilerOptions())
) {
logger.log('Detected that this is not a Svelte project, abort patching TypeScript');
return info.languageService;
}
Expand Down
Loading