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

modules that neither export nor import shouldn't be wrapped in define-call #4989

Closed
jrieken opened this issue Jun 17, 2022 · 15 comments · Fixed by #4993
Closed

modules that neither export nor import shouldn't be wrapped in define-call #4989

jrieken opened this issue Jun 17, 2022 · 15 comments · Fixed by #4993
Labels
Milestone

Comments

@jrieken
Copy link

jrieken commented Jun 17, 2022

Describe the bug

This is maybe controversial and something we can tackle on our end but it is different from how tsc behaves

Input code

namespace foo {
  someglobal.fn();
}

Config

{
  "$schema": "http://json.schemastore.org/swcrc",
  "exclude": "\\.js$",
  "jsc": {
    "parser": {
      "syntax": "typescript",
      "tsx": false,
      "decorators": true
    },
    "target": "es2020",
    "loose": false,
    "minify": {
      "compress": false,
      "mangle": false
    }
  },
  "module": {
    "type": "amd",
    "noInterop": true
  },
  "minify": false
}

Playground link

https://play.swc.rs/?version=1.2.203&code=H4sIAAAAAAAAA8tLzE0tLkhMTlVIy89XqOZSUCjOz01Nz8lPSszRS8vT0LTmqgUAxm2peCQAAAA%3D&config=H4sIAAAAAAAAA0WOTQ6DQAhG70JcNmpceoPewc1kxL84wwQw1RjvXqzR7sjj%2Bx7skIkfMDioYVBNdVFMQjG%2FoCgx5sR9IR%2FPHl6Aq5%2BXFi3dNPkkmaFJPNQ7JMeCfE6yRXWrRXRLKJ7HpBZTMdS5WfAFLXpiZ3KBWnnBw9aOe1QroVRlVVphJhJ8KmGMY7edek8hMYr8Vy728508zBWoXU6w%2Fz4wpwutCSO9oyJTeo7e0qv5BTnH4YgMAQAA

Expected behavior

With tsc the define-call is skipped because the code doesn't export or import anything.

https://www.typescriptlang.org/play?module=2#code/HYQwtgpgzgDiDGEAEAzA9mpBvAUEpUakA5gDZoBGIpAdCsABQCUA3DgL5A

Actual behavior

The code is wrapped in a define call making it hard ti transpile code that actually bootstraps the AMD logic

Version

1.2.203

Additional context

relates to, but not strictly needed for, microsoft/vscode#150025

@jrieken jrieken added the C-bug label Jun 17, 2022
@magic-akari
Copy link
Member

Input

const hello = "world";
console.log(hello);
define((function () { 'use strict';

	const hello = "world";
	console.log(hello);

}));

@kdy1 kdy1 added this to the Planned milestone Jun 17, 2022
@kdy1
Copy link
Member

kdy1 commented Jun 18, 2022

I think we should follow behavior of tsc for ts-specific things

TSC: https://www.typescriptlang.org/play?#code/HYQwtgpgzgDiDGEAEAzA9mpBvAUEpUakA5gDZoBGIpAdCsABQCUA3DgL5A

@magic-akari
Copy link
Member

You can use isModule: "unknown" to make swc behave identical as tsc. (While allowing other files to be handled as module)

https://play.swc.rs/?version=1.2.203&code=H4sIAAAAAAAAA8tLzE0tLkhMTlVIy89XqOZSUCjOz01Nz8lPSszRS8vT0LTmqgUAxm2peCQAAAA%3D&config=H4sIAAAAAAAAA0WOTQ6CQAxG79KwNGBccgMX3sDNZKgIMu2kLRFCuLvFH9w1r9%2F32gUKjXdMAWq4m%2BW6qnplKj9QjQVLlrbSZ5QIB8ApDmODnr5ey14LR71GqBfIQRRlm3QmC5NHbM6oUbpsHjN1dAuD4gEajCzB5Qq1yYirr4O0aF5CPR1PRy8MzIp7JXXU3eZNHzllQdX%2FKlA7%2FJKruxI34waW9wfuDKlxIfGZDIXzfvQn%2FYo6vXybMNKD%2BEmwvgDqvBMKIQEAAA%3D%3D

But .swcrc does not support this field...

How do you invoke swc?

cannot agree more.
If there is neither an export nor an import in the file, TypeScript assumes it is a script.

@kdy1
Copy link
Member

kdy1 commented Jun 18, 2022

@magic-akari Do you mean we should support it in .swcrc?

@magic-akari
Copy link
Member

magic-akari commented Jun 18, 2022

@jrieken
Copy link
Author

jrieken commented Jul 11, 2022

@kdy1 Is https://swc.rs/docs/configuration/compilation#multiple-entries actually supported inside .swcrc? E.g declaring an array of configs in it I get errors complaining about invalid JSON

@kdy1
Copy link
Member

kdy1 commented Jul 11, 2022

Yes, but note that regex syntax is one of rust regex crate

@kdy1
Copy link
Member

kdy1 commented Jul 11, 2022

@jrieken Can you share your .swcrc?

@jrieken
Copy link
Author

jrieken commented Jul 12, 2022

@kdy1 there you go. It looks like it failing as soon as I put the config-object into an array (e.g works when it's a top-level object)

[
	{
		"test": "\\.ts$",
		"jsc": {
			"parser": {
				"syntax": "typescript",
				"tsx": false,
				"decorators": true
			},
			"target": "es2020",
			"loose": false,
			"minify": {
				"compress": false,
				"mangle": false
			}
		},
		"module": {
			"type": "amd",
			"noInterop": true
		},
		"minify": false
	}
]

@kdy1
Copy link
Member

kdy1 commented Jul 13, 2022

image

Hmm....
Seems like deserialization itself is successful

@jrieken
Copy link
Author

jrieken commented Jul 13, 2022

Funny... I see the error below running npx swc --config-file .swcrc src --copy-files --out-dir out in this branch (https://github.com/microsoft/vscode/tree/joh/swc)

jrieken@jrieken-m1-pro vscode % npx swc --config-file .swcrc src --copy-files --out-dir out
failed to process input file

Caused by:
    0: failed to read swcrc file (src/bootstrap-amd.js)
    1: failed to process config file
    2: .swcrc exists but not matched

@kwonoj
Copy link
Member

kwonoj commented Jul 15, 2022

@jrieken I.. don't think we support top level array as config unless if I miss something? Please allow me a dumb q, any specific reason you need to use array instead of an object for the swcrc config?

@kdy1
Copy link
Member

kdy1 commented Jul 16, 2022

@kwonoj We support top level array.
The screenshot is about the array config test

@swc-bot
Copy link
Collaborator

swc-bot commented Oct 16, 2022

This closed issue has been automatically locked because it had no new activity for a month. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you.

@swc-project swc-project locked as resolved and limited conversation to collaborators Oct 16, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Development

Successfully merging a pull request may close this issue.

5 participants