feat(es/transforms): Add module.outFileExtension
#9784
Conversation
🦋 Changeset detectedLatest commit: 4eb8ad0 The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
|
hanseltime
force-pushed
the
feat-out-file-extension-resolution
branch
from
3ef730f to
9a35817
Compare
CodSpeed Performance ReportMerging #9784 will degrade performances by 4.89%Comparing Summary
Benchmarks breakdown
|
|
👍 Dependency issues cleared. Learn more about Socket for GitHub ↗︎ This PR previously contained dependency changes with security issues that have been resolved, removed, or ignored. |
|
Noting here a few things that I'm unsure on:
Thanks in advance! |
crates/swc/src/config/mod.rs
Outdated
| @@ -1733,6 +1750,7 @@ fn build_resolver( | |||
| mut base_url: PathBuf, | |||
| paths: CompiledPaths, | |||
| resolve_fully: bool, | |||
| file_extension: &String, | |||
There was a problem hiding this comment.
I think clippy will be angry at this
There was a problem hiding this comment.
| file_extension: &String, | |
| file_extension: &str, |
There was a problem hiding this comment.
Ah thanks. I have updated it!
| @@ -21,6 +22,9 @@ pub struct Config { | |||
|
|
|||
| #[serde(default)] | |||
There was a problem hiding this comment.
@kdy1 - I didn't add this attribute. Did you mean the inner Config attribute below it or were you hoping I would clean this up? Also, I'm assuming you would want this changed across all Module Shapes or just this one?
There was a problem hiding this comment.
I mean the whole resolve_fully: bool. It's now duplicated as you added a field below.
There was a problem hiding this comment.
Oh I see. Thanks for clarifying. Okay, I've updated that for the ESModuleConfig and the SystemJS Config.
| pub fn default_js_ext() -> String { | ||
| "js".to_string() | ||
| } |
There was a problem hiding this comment.
| pub fn default_js_ext() -> String { | |
| "js".to_string() | |
| } | |
| impl Config { | |
| pub fn default_js_ext() -> String { | |
| "js".to_string() | |
| } | |
| } |
There was a problem hiding this comment.
I've switched the function to an impl on Config and all calls to the function to use the Config::default_js_ext() as well.
hanseltime
force-pushed
the
feat-out-file-extension-resolution
branch
from
252afff to
258b834
Compare
hanseltime
force-pushed
the
feat-out-file-extension-resolution
branch
from
258b834 to
efe8f77
Compare
kdy1
changed the title
module.outFileExtension
Description:
Summary of Changes:
outFileExtensionproperty to every module configuration option struct"js"checks to use the new parameter@swc/coresnapshot tests for each module usingresolveFullyandoutFileExtension(Please check the rendered output of the snapshots to make sure you don't have any issues with it).@swc/corejest config to use explicit module names (this was because the mapping util in jest was exploding when it would try to load in the .node files that were over 512MB)Note
This is a rudimentary fix to: #3067 (comment). I had to implement it locally since I could not find a workaround in my timeline.
I do know that @kdy1 said they would work on it a few days ago, so I am opening this to either provide some visibility to a solution that works for me if they haven't started or to let them switch to PR critique if that feels more valuable for their time on this issue. Let me know if this should be closed due to a better implementation already being in the works.
Related issue: