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

Minor issues with preprocessed TypeScript #665

Open
nstuyvesant opened this issue Feb 15, 2025 · 0 comments
Open

Minor issues with preprocessed TypeScript #665

nstuyvesant opened this issue Feb 15, 2025 · 0 comments

Comments

@nstuyvesant
Copy link

nstuyvesant commented Feb 15, 2025

Describe the design limitations

  1. svelte-preprocess does not remove lang="ts" from generated files - only lang="typescript". This differs from vitePreprocess as it expects lang="ts" and is confusing for users who might need svelte-preprocess after starting with vitePreprocess.
  2. Generated svelte components do not have a line break following the opening <script> tag.
  3. Generated svelte components do not have the generics attribute removed from <script> tags.
  4. Generated svelte components do not apply prettier configuration. While we can manually run prettier on the dist folder but it would be nice if svelte-preprocess followed the configured rules.

To Reproduce

  1. Create a Svelte component with the following code:
<script lang="typescript" generics="T extends BaseChartOptions">
	import { onMount, afterUpdate, onDestroy, createEventDispatcher } from 'svelte'
	import type { Charts, ChartConfig, BaseChartOptions, ChartTabularData } from '@carbon/charts'

	interface DispatchedEvents {
		load: null
		update: { data: ChartTabularData; options: T }
		destroy: null
	}

	type CoreChartClass = new (holder: HTMLDivElement, chartConfigs: ChartConfig<T>) => Charts

	const chartHolderCssClass = 'cds--chart-holder' // Used by Carbon Charts CSS
	export let data: ChartTabularData = [] // Chart data, default is empty
	export let options: T = {} as T // Specific chart option type, default is empty
	export let Chart: CoreChartClass // Used to instantiate next property
	export let chart: Charts | undefined // Instance of the chart class
	export let ref: HTMLDivElement | undefined // Binding to chart 'holder' in template below
	export let id = `chart-${Math.random().toString(36)}` // id for chart holder element

	const dispatch = createEventDispatcher<DispatchedEvents>()

	onMount(() => {
		try {
			chart = new Chart(ref as HTMLDivElement, { data, options })
			dispatch('load')
		} catch (error) {
			console.error('Failed to initialize chart:', error)
		}
	})

	afterUpdate(() => {
		if (chart) {
			try {
				chart.model.setData(data)
				chart.model.setOptions(options)
				dispatch('update', { data, options })
			} catch (error) {
				console.error('Failed to update chart:', error)
			}
		}
	})

	onDestroy(() => {
		if (chart) {
			dispatch('destroy')
			// Like core's Chart.destroy() but keeps div chart holder bound to ref as it's part of this template
			chart.components.forEach(component => component.destroy())
			chart.model.set({ destroyed: true }, { skipUpdate: true })
			chart = undefined
		}
	})
</script>

<div {id} bind:this={ref} class={chartHolderCssClass} {...$$restProps}></div>
  1. Build with svelte-preprocess.
  2. Look at output...
<script generics="T extends BaseChartOptions">import { onMount, afterUpdate, onDestroy, createEventDispatcher } from 'svelte';
const chartHolderCssClass = 'cds--chart-holder'; // Used by Carbon Charts CSS
export let data = []; // Chart data, default is empty
export let options = {}; // Specific chart option type, default is empty
export let Chart; // Used to instantiate next property
export let chart; // Instance of the chart class
export let ref; // Binding to chart 'holder' in template below
export let id = `chart-${Math.random().toString(36)}`; // id for chart holder element
const dispatch = createEventDispatcher();
onMount(() => {
    try {
        chart = new Chart(ref, { data, options });
        dispatch('load');
    }
    catch (error) {
        console.error('Failed to initialize chart:', error);
    }
});
afterUpdate(() => {
    if (chart) {
        try {
            chart.model.setData(data);
            chart.model.setOptions(options);
            dispatch('update', { data, options });
        }
        catch (error) {
            console.error('Failed to update chart:', error);
        }
    }
});
onDestroy(() => {
    if (chart) {
        dispatch('destroy');
        // Like core's Chart.destroy() but keeps div chart holder bound to ref as it's part of this template
        chart.components.forEach(component => component.destroy());
        chart.model.set({ destroyed: true }, { skipUpdate: true });
        chart = undefined;
    }
});
</script>

<div {id} bind:this={ref} class={chartHolderCssClass} {...$$restProps}></div>

Expected behavior

  1. Expected generics="T extends BaseChartOptions" to be removed from line 1.
  2. Expected there to be a line break before the first import statement.
  3. Expected lang="ts" would be removed (my example uses lang="typescript" which is removed)
  4. Expected formatting to use prettier

Information about your project:

  • Your browser and the version: Google Chrome 133

  • Your operating system: macOS 15.3.1

  • svelte-preprocess version: 6.0.3

  • Svelte 5.20.1 (compatibility mode), SvelteKit 2.17.2 (building with svelte-package)

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

No branches or pull requests

1 participant