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

test: add tests for nativeClassProperties #64

Merged
merged 1 commit into from
Aug 14, 2024

Conversation

marco-ippolito
Copy link
Member

No description provided.

@marco-ippolito marco-ippolito force-pushed the feat/test-nativeClassProperties branch from 80190fa to d06ced4 Compare August 14, 2024 14:03
@marco-ippolito marco-ippolito merged commit 341be22 into main Aug 14, 2024
8 checks passed
@marco-ippolito marco-ippolito deleted the feat/test-nativeClassProperties branch August 14, 2024 14:06
@robpalme
Copy link

The test case is wrong. The desired outcome is for field initializers to remain as field initializers.

I'm guessing the transforms list needs an extra option: useDefineForClassFields: true.

@marco-ippolito
Copy link
Member Author

marco-ippolito commented Aug 14, 2024

this is the updated test

test("test native class properties", (t) => {
	const inputCode = `
	class TypeScriptParameterProperties {

    initializedField = this.pp;
    constructor(public pp: any) {
        console.log('finally');
     }
	}`;
	const { code } = transformSync(inputCode, {
		mode: "transform",
		sourceMap: true,
		transform: {
			verbatimModuleSyntax: true,
			nativeClassProperties: true,
			useDefineForClassFields: true,
		},
	});
	console.log(code)
	t.assert.snapshot(code);
});

output:

class TypeScriptParameterProperties {
    pp;
    initializedField;
    constructor(pp){
        this.pp = pp;
        this.initializedField = this.pp;
        console.log('finally');
    }
}

@robpalme is it correct?
I actually noticed that nativeClassProperties is not present in 1.7.11 of the version that we use @kdy1, @magic-akari

@robpalme
Copy link

The test output is incorrect. So we're not picking up the desired new behavior yet.

@magic-akari
Copy link

The current module relationship is
binding_typescript_wasm -> swc_fast_ts_strip -> swc_ecma_transforms_typescript

Now, the swc_ecma_transforms_typescript module has been updated, but no new version has been released yet,
all dependencies are still referencing the old version.

We may need @kdy1 to explain the compilation and update process.

@robpalme
Copy link

@magic-akari do you know if today's @swc/wasm-typescript v1.7.12 contains this new option?

@magic-akari
Copy link

@magic-akari do you know if today's @swc/wasm-typescript v1.7.12 contains this new option?

I don't know.


There are methods to ensure the use of a local build over crate packages. For details, refer to the script in this CI
https://github.com/swc-project/swc/blob/05961eb018e2e76ed5ef95de9bad923b2fe1df88/.github/workflows/CI.yml#L147-L152

I'm not certain if this is the recommended method.

@marco-ippolito
Copy link
Member Author

maybe @kdy1 knows?

@kdy1
Copy link
Member

kdy1 commented Aug 19, 2024

Sorry, I missed the email. I'll publish swc_ecma_transforms_typescript and swc_fast_ts_strip right away.

@kdy1
Copy link
Member

kdy1 commented Aug 19, 2024

I published those crates and triggered publish of v1.7.14

@marco-ippolito
Copy link
Member Author

thank you!

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

Successfully merging this pull request may close these issues.

4 participants