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.

Sign up for GitHub

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

Jump to bottom

fix(js): update swc options so path mappings can work in all environments #16390

Merged
merged 1 commit into from
Apr 19, 2023
Merged

fix(js): update swc options so path mappings can work in all environments #16390

merged 1 commit into from
Apr 19, 2023

Conversation

jaysoo
Copy link
Member

@jaysoo jaysoo commented Apr 19, 2023 •
edited
Loading

This PR updates @nx/js:swc executor so it properly compiles with jsc.paths set in .swcrc for all environments. Another PR is failing because the compiled code has wrong path mapping. #16284

Details:

  1. It looks like we're setting jsc.baseUrl with the wrong default. It should be . since cwd is set to the project root. That way, when mappings are used in output, it will be relative to the project root. Removing that whole logic and relying on users to set baseUrl seems more reliable. Also with local testing (macos, debian) it doesn't look like baseUrl is needed by swc at all (at least on latest version).
  2. Passing both --no-swc and --config-file together is causing wrong paths in output as well for some reason (able to replicate this in CI with just npx swc and not Nx, but not locally). Since we're passing the config file anyway, we don't need the --no-swc option to find the file.

Current Behavior

Expected Behavior

Related Issue(s)

Fixes #

@vercel
Copy link

vercel bot commented Apr 19, 2023 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment
Name Status Preview Comments Updated (UTC)
nx-dev ⬜️ Ignored (Inspect) Apr 19, 2023 5:02am

@jaysoo jaysoo marked this pull request as ready for review April 19, 2023 04:05
@jaysoo jaysoo force-pushed the fix/js-swc-paths branch from e3e6331 to f148e7d Compare April 19, 2023 04:07
@nx-cloud
Copy link

nx-cloud bot commented Apr 19, 2023 •
edited
Loading

☁️ Nx Cloud Report

CI is running/has finished running commands for commit 42205fa. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this branch

✅ Successfully ran 8 targets

Sent with 💌 from NxCloud.

@jaysoo jaysoo force-pushed the fix/js-swc-paths branch from f148e7d to c9dc569 Compare April 19, 2023 05:01
@jaysoo jaysoo force-pushed the fix/js-swc-paths branch from c9dc569 to 42205fa Compare April 19, 2023 05:02
nartc
nartc reviewed Apr 19, 2023
View reviewed changes
e2e/js/src/js-swc.test.ts
json.jsc.paths = {
'src/*': ['src/*'],
'~/*': ['./src/*'],
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

comment: so the key is to use relative path only for path mappings with swc config file?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, and the baseUrl is only added to pass CI. It's not needed locally (MacOS/Debian), but seems to be needed in CI even with the same swc versions.

@jaysoo jaysoo merged commit ab609a2 into nrwl:master Apr 19, 2023
@jaysoo jaysoo deleted the fix/js-swc-paths branch April 19, 2023 14:38
meeroslav added a commit to meeroslav/nx that referenced this pull request Apr 20, 2023
@meeroslav
chore(repo): merge master to branch
5f5370c 
Squashed commit of the following:

commit a51ac58
Author: Miroslav Jonaš <[email protected]>
Date:   Thu Apr 20 09:16:45 2023 +0200

    fix(core): do not expect each package to have hoisted version in lockfile (nrwl#16393)

commit 4a4fc19
Author: Miroslav Jonaš <[email protected]>
Date:   Thu Apr 20 09:16:32 2023 +0200

    fix(core): enforce fixed versions only for apps with createPackageJson (nrwl#16398)

commit 8400484
Author: Emily Xiong <[email protected]>
Date:   Wed Apr 19 20:36:11 2023 -0400

    feat(core): update create-nx-plugin to generate cli library (nrwl#15994)

commit 16e4061
Author: Craigory Coppola <[email protected]>
Date:   Wed Apr 19 18:25:41 2023 -0400

    feat(devkit): bump support range to be 15 <= x <= 17 (nrwl#16421)

commit fc07794
Author: Craigory Coppola <[email protected]>
Date:   Wed Apr 19 18:07:56 2023 -0400

    feat(core): remove deprecated affected commands (nrwl#16349)

commit a798576
Author: Craigory Coppola <[email protected]>
Date:   Wed Apr 19 17:02:34 2023 -0400

    fix(misc): pass e2eTestRunner to child preset generator (nrwl#16414)

commit 338dc64
Author: Nicholas Cunningham <[email protected]>
Date:   Wed Apr 19 13:29:31 2023 -0600

    feat(nextjs): Add standalone Nextjs option to react selection when running CNW (nrwl#16317)

commit 16e115f
Author: Emily Xiong <[email protected]>
Date:   Wed Apr 19 15:03:36 2023 -0400

    fix(react-native): log errors for pod install (nrwl#16386)

commit 0fc3d44
Author: FrozenPandaz <[email protected]>
Date:   Wed Apr 19 14:24:10 2023 -0400

    chore(misc): publish 16.0.0-beta.4

commit f7ca6e6
Author: Craigory Coppola <[email protected]>
Date:   Wed Apr 19 14:21:30 2023 -0400

    chore(misc): update migration version for workspace-generator (nrwl#16417)

commit b3ca503
Author: Craigory Coppola <[email protected]>
Date:   Wed Apr 19 14:12:09 2023 -0400

    fix(core): hide duplicate log entry when displaying generator help (nrwl#16353)

commit 9fa6d41
Author: Craigory Coppola <[email protected]>
Date:   Wed Apr 19 14:06:57 2023 -0400

    fix(misc): mark workspace-lint as deprecated in yargs config (nrwl#16385)

commit 9c9abaa
Author: Emily Xiong <[email protected]>
Date:   Wed Apr 19 14:03:45 2023 -0400

    fix(react-native): remove listDevices default value for run-android (nrwl#16410)

commit 1743ff1
Author: Craigory Coppola <[email protected]>
Date:   Wed Apr 19 14:03:00 2023 -0400

    feat(core): add migration to update workspace generators to a local plugin (nrwl#12700)

commit f04f316
Author: Jack Hsu <[email protected]>
Date:   Wed Apr 19 14:00:24 2023 -0400

    feat(react): add rspack experimental support (nrwl#16252)

commit 1ff450b
Author: Jason Jean <[email protected]>
Date:   Wed Apr 19 13:43:44 2023 -0400

    fix(core): fix devkit compatibility (nrwl#16413)

commit 6677a9c
Author: Leosvel Pérez Espinosa <[email protected]>
Date:   Wed Apr 19 18:32:51 2023 +0100

    feat(misc): expose nx init command flags (nrwl#16287)

commit 5d7ad34
Author: Leosvel Pérez Espinosa <[email protected]>
Date:   Wed Apr 19 18:22:52 2023 +0100

    feat(testing): bump ts-jest to 29.1.0 to support typescript 5.x (nrwl#16401)

commit a8e85da
Author: Dominik Pieper <[email protected]>
Date:   Wed Apr 19 19:01:50 2023 +0200

    feat(testing): export cypressE2EConfigurationGenerator (nrwl#16394)

commit 1ad7e3d
Author: Caleb Ukle <[email protected]>
Date:   Wed Apr 19 11:55:11 2023 -0500

    fix(testing): nullcheck polyfills in ng component testing (nrwl#16411)

commit c963393
Author: Katerina Skroumpelou <[email protected]>
Date:   Wed Apr 19 19:34:38 2023 +0300

    fix(storybook): dont use swc addon on sb7 and format (nrwl#16408)

commit c8960b7
Author: Miroslav Jonaš <[email protected]>
Date:   Wed Apr 19 17:13:10 2023 +0200

    fix(linter): don't confuse buildable libs for secondary ng entry point (nrwl#16367)

commit 2734401
Author: FrozenPandaz <[email protected]>
Date:   Wed Apr 19 11:10:19 2023 -0400

    chore(misc): publish 16.0.0-beta.3

commit da158bc
Author: Jason Jean <[email protected]>
Date:   Wed Apr 19 10:59:58 2023 -0400

    feat(angular): replace usages of @nrwl with @nx (nrwl#16377)

    Co-authored-by: Colum Ferry <[email protected]>

commit 76dfc62
Author: Jason Jean <[email protected]>
Date:   Wed Apr 19 10:59:41 2023 -0400

    feat(web): replace usages of @nrwl with @nx (nrwl#16376)

commit 1780aac
Author: Jack Hsu <[email protected]>
Date:   Wed Apr 19 10:57:34 2023 -0400

    feat(react): add @nx/web as a dependency since it is needed by cypress/etc. (nrwl#16407)

commit 52acf07
Author: Jason Jean <[email protected]>
Date:   Wed Apr 19 10:40:37 2023 -0400

    fix(angular): fix resolve builder in ngcli adapter (nrwl#16375)

commit ab609a2
Author: Jack Hsu <[email protected]>
Date:   Wed Apr 19 10:38:01 2023 -0400

    fix(js): update swc options so path mappings can work in all environments (nrwl#16390)

commit 157b35b
Author: Katerina Skroumpelou <[email protected]>
Date:   Wed Apr 19 17:33:10 2023 +0300

    fix(storybook): do not override existing v7 on init (nrwl#16405)
FrozenPandaz pushed a commit that referenced this pull request Apr 24, 2023
@jaysoo @FrozenPandaz
fix(js): update swc options so path mappings can work in all environm…
c1cf1f9 
…ents (#16390)

(cherry picked from commit ab609a2)
FrozenPandaz pushed a commit that referenced this pull request Apr 24, 2023
@jaysoo @FrozenPandaz
fix(js): update swc options so path mappings can work in all environm…
975eaca 
…ents (#16390)

(cherry picked from commit ab609a2)
@github-actions
Copy link

This pull request has already been merged/closed. If you experience issues related to these changes, please open a new issue referencing this pull request.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 25, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@nartc nartc nartc approved these changes

@meeroslav meeroslav meeroslav approved these changes

@ndcunningham ndcunningham Awaiting requested review from ndcunningham

Assignees

@jaysoo jaysoo

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@jaysoo @meeroslav @nartc