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

AMD generated code misses local require-function #4718

Closed
jrieken opened this issue May 20, 2022 · 4 comments · Fixed by #4727
Closed

AMD generated code misses local require-function #4718

jrieken opened this issue May 20, 2022 · 4 comments · Fixed by #4727
Labels
Milestone

Comments

@jrieken
Copy link

jrieken commented May 20, 2022

Describe the bug

With AMD there is the global and local require-function (more here). The latter knows the url of the current module and has a toUrl-util enabling modules to create urls of related resources. Think of this as the ancestor of import.meta.url. This doesn't seem to be supported with SWC because the define functions it generates don't spell out the "require" special import

Input code

function fff() {
	require.toUrl("");
}

Config

{
  "jsc": {
    "parser": {
      "syntax": "typescript",
      "tsx": false
    },
    "target": "es2020"
  },
  "module": {
    "type": "amd"
  }
}

Playground link

https://play.swc.rs/?version=1.2.186&code=H4sIAAAAAAAAA0srzUsuyczPU0hLS9PQVKjm4ixKLSzNLErVK8kPLcrRUFLStOaqBQAX%2B2D7JgAAAA%3D%3D&config=H4sIAAAAAAAAA0WOMQ6DMAxF7%2BKZATFygw49hBUMSpXEkW2kRoi7N0GhbNbz%2F88%2B4KMO5gMyipK0SUsy%2FMIMVjKpE58NBjCtaMWgNMBCjgWNRWE22RspCaN3r5hZrNOzllA2sqoincZprJrArPQXRZ%2F8WtpRxzELqT4rTFu4k2d1RV72Bo7rr%2BrEuMD5OHrP67sHrx9%2B9tLJVOAAAAA%3D

Expected behavior

The define-call's dependency-array should contain the special "require"-module (similar to the special "exports"-module) and also the callback should be define a require argument so that module use their local require-call, not the global one

define(["require"], function(require) {
    "use strict";
    function fff() {
        require.toUrl("");
    }
});

See TypeScript Playground: https://www.typescriptlang.org/play?jsx=0&module=2#code/JYWwDg9gTgLgBAbwIYF84DMoRHA5E3AKEPQFcA7AYxmAnI3XQAoBKRQgSCgFMBHU4DwB0MCAFUoAGyYAiGSwDchFEA

Actual behavior

The define-call should depend on require, either always or by knowing if any dependency onto require exists

Version

1.2.186

Additional context

Thanks for this great project! Related to microsoft/vscode#150025

@kwonoj
Copy link
Member

kwonoj commented May 20, 2022

@jrieken mind check ts playground link? I tried open couple of times and it shows this instead:
Screen Shot 2022-05-20 at 12 29 45 PM

@kwonoj
Copy link
Member

kwonoj commented May 20, 2022

Well, I could run tsc locally to see its behavior.

@jrieken
Copy link
Author

jrieken commented May 21, 2022

Well, I could run tsc locally to see its behavior.

Great.

One small clarification: The local require-call is needed for the require.toUrl but also for normal, async imports, e.g

function fff() {
	import('./sibling').then(sib => {}) 
       // ☝️needs the local require which knows how to resolve the relative path
}

I'd say it's fair to always add the local require but when looking for actual usages than both use-cases should be considered. Thanks!

@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
4 participants