refactor: use consistent build process across packages #3456

nolanlawson · 2023-04-11T22:33:15Z

Details

Partially addresses #3445

Does this pull request introduce a breaking change?

✅ No, it does not introduce a breaking change.

Update: breaking changes have been removed! I've added backwards compat for usages like this:

require('@lwc/synthetic-shadow/dist/synthetic-shadow.js')
require('@lwc/engine-server/dist/engine-server.js')
require('@lwc/compiler/dist/commonjs/transformers/transformer')

I've also added a TODO so that hopefully we can remove this when we're ready to release LWC v3.0.0.

Does this pull request introduce an observable change?

⚠️ Yes, it does include an observable change.

The dist/ files have changed, and some exports have been added, so it may be detectable in certain use cases. But this is not detectable to LWC component authors on core – only to npm package consumers.

BREAKING CHANGE

nolanlawson · 2023-04-11T22:40:16Z

scripts/tasks/check-and-rewrite-package-json.js

This script is the main magic; everything else is basically just details.

nolanlawson · 2023-04-11T22:42:43Z

nx.json

+    },
+    "namedInputs": {
+        "sharedGlobals": ["{workspaceRoot}/scripts/rollup/rollup.config.js"],
+        "default": ["{projectRoot}/**/*", "sharedGlobals"]


This tells NX that when this file changes, it should bust the cache

nolanlawson · 2023-04-11T22:52:15Z

packages/@lwc/template-compiler/src/index.ts

@@ -29,6 +29,9 @@ export function parse(source: string, config: Config = {}): TemplateParseResult
    return parseTemplate(source, state);
 }

+// Export as a named export as well for easier importing in certain environments (e.g. Jest)
+export { compile };
+


I needed this to unbreak our Jest tests. Jest is weird about default exports.

I'm wondering if this will break anything else in the DX ecosystem, if they import @lwc/template-compiler directly. I'm fine with this change, but we should keep an eye out for issues that might arise because of this change.

I'm hoping we'll be fine, and that this is just an internal issue with TypeScript+Jest on our source code, but it's impossible to know for sure.

FWIW the current package uses exports.default = compile in the dist CJS file, and the new dist file does the same thing.

nolanlawson · 2023-04-11T22:53:41Z

packages/@lwc/wire-service/package.json

    "devDependencies": {
        "@lwc/engine-core": "2.43.0",
        "@lwc/shared": "2.43.0"
    },
    "lwc": {
        "modules": [
-            {
-                "name": "wire-service",


Technically I'm breaking import 'wire-service', but we shouldn't support this anyway because of dependency confusion. (Luckily we are not vulnerable because we squatted the package.)

Also I doubt anybody is using this.

nolanlawson · 2023-04-11T22:54:39Z

packages/lwc/scripts/utils/helpers.js

@@ -15,7 +15,7 @@ function createDir(dir) {
 }

 function getEs6ModuleEntry(pkg) {
-    const pkgFilePath = require.resolve(`${pkg}/package.json`);
+    const pkgFilePath = path.resolve(__dirname, '../../../', pkg, './package.json');


I changed this because originally I was using package exports, which breaks if you try to require an unexposed file. I think it's better to just use path.resolve anyway though.

nolanlawson · 2023-04-12T19:27:38Z

~~This PR breaks @lwc/jest-preset, looks like we can solve it here.~~

No more breakages!

nolanlawson · 2023-04-12T19:43:47Z

~~Cataloguing some of the errors I'm seeing, so those desperately Googling can wind up on this PR:~~

Update: no more breakages!

Cannot find module '@lwc/compiler/dist/commonjs/transformers/transformer'

Cannot find module '@lwc/synthetic-shadow/dist/synthetic-shadow.js'

Cannot find module '@lwc/engine-server/dist/engine-server.js'

~~(Hint: the solution is to update all your deps, and update your require()s/imports to the new location.)~~

ekashida · 2023-04-26T16:35:27Z

A lot of nice changes in here, should we separate out the non-breaking changes from the breaking changes so that the non-breaking ones are not held up here?

nolanlawson · 2023-05-01T18:18:02Z

I was thinking the same thing, yeah. I can rework this so that there are no breaking changes, and then add TODOs for the breaking changes.

nolanlawson · 2023-05-09T21:48:30Z

154 lines of code removed! I'm happy about this.

@ekashida The breaking changes have been removed – at least, downstream Nucleus tests are passing. Can you take another look?

nolanlawson · 2023-05-09T22:03:22Z

packages/@lwc/template-compiler/README.md

@@ -11,7 +11,7 @@ yarn add --dev @lwc/template-compiler
 ## Usage

 ```js
-import compile from '@lwc/template-compiler';
+import { compile } from '@lwc/template-compiler';


Named exports are way more compatible across the JS ecosystem than default exports. We should just recommend people to use this, even if we still support the default exports.

divmain

This is fantastic!

packages/@lwc/compiler/src/transformers/template.ts

divmain · 2023-05-11T19:57:35Z

packages/@lwc/features/src/index.ts

Also cleanup?

The root source file of each package is now named index.js. @lwc/features was inconsistent – its root source file was called flags.ts.

divmain · 2023-05-11T20:00:01Z

packages/@lwc/template-compiler/src/index.ts

@@ -29,6 +29,9 @@ export function parse(source: string, config: Config = {}): TemplateParseResult
    return parseTemplate(source, state);
 }

+// Export as a named export as well for easier importing in certain environments (e.g. Jest)
+export { compile };
+


I'm wondering if this will break anything else in the DX ecosystem, if they import @lwc/template-compiler directly. I'm fine with this change, but we should keep an eye out for issues that might arise because of this change.

scripts/rollup/rollup.config.js

nolanlawson · 2023-05-11T22:29:23Z

Boom! 💥

ekashida · 2023-05-16T02:27:30Z

Sorry for the late review. This is awesome, thanks!

nolanlawson · 2023-05-16T19:32:46Z

packages/@lwc/shared/package.json

    "license": "MIT",
-    "scripts": {
-        "build": "rollup --config ./scripts/rollup/rollup.config.js",
-        "postbuild": "node ../../../scripts/tasks/verify-treeshakable.js ./dist/index.js",


Accidentally removed this

nolanlawson added 2 commits April 11, 2023 15:28

fix: use consistent build process across packages

7bb8d01

BREAKING CHANGE

fix: newlines

e0c903c

nolanlawson commented Apr 11, 2023

View reviewed changes

fix: pkg.json i did not mean to change

393f07c

nolanlawson commented Apr 11, 2023

View reviewed changes

nolanlawson added 2 commits April 11, 2023 15:57

fix: add keywords back in

77c50d8

fix: typo in comment

ae28f47

nolanlawson marked this pull request as ready for review April 11, 2023 22:59

nolanlawson marked this pull request as draft April 11, 2023 23:25

fix: fix rollup script

afc0779

nolanlawson added this to the 3.0.0 milestone Apr 12, 2023

nolanlawson added the nomerge label Apr 12, 2023

nolanlawson changed the title ~~fix: use consistent build process across packages~~ fix: use consistent build process across packages (BREAKING CHANGE) Apr 12, 2023

fix: remove "./" as it confuses npm during publishing

5a466c4

nolanlawson mentioned this pull request Apr 12, 2023

fix: future-proof against changes to synthetic shadow salesforce/lwc-test#179

Merged

nolanlawson added 7 commits May 8, 2023 16:20

fix: tweak script

e93ee62

fix: no need for braces

6229d5c

fix: add backwards compat

11912aa

Merge remote-tracking branch 'origin/master' into nolan/package-reform-2

3a8ec8d

fix: backwards compat

e83ff66

fix: prefer named imports to default imports

6e1dc2c

fix: add more backwards compat

2fbfb14

nolanlawson changed the title ~~fix: use consistent build process across packages (BREAKING CHANGE)~~ refactor: use consistent build process across packages (BREAKING CHANGE) May 9, 2023

nolanlawson added 6 commits May 9, 2023 13:54

refactor: refactor script

37269a6

fix: use node: prefix

e65f997

fix: simplify tsconfig

d96251e

fix: simplify tsconfig

13af442

fix: simplify tsconfig

a0bf6df

fix: simplify tsconfig

3809231

nolanlawson marked this pull request as ready for review May 9, 2023 21:25

fix: typo

8d23ba6

nolanlawson removed the nomerge label May 9, 2023

nolanlawson mentioned this pull request May 9, 2023

Expose consistent dist files across packages #3445

Closed

nolanlawson removed this from the 3.0.0 milestone May 9, 2023

nolanlawson commented May 9, 2023

View reviewed changes

Merge remote-tracking branch 'origin/master' into nolan/package-reform-2

afaf07f

divmain approved these changes May 11, 2023

View reviewed changes

nolanlawson added 2 commits May 11, 2023 13:51

fix: switch to named import

8e6ecb6

fix: respond to PR comment

089cd4a

nolanlawson merged commit cfb0424 into master May 11, 2023

nolanlawson deleted the nolan/package-reform-2 branch May 11, 2023 22:29

nolanlawson mentioned this pull request May 11, 2023

chore: remove unused build script #3493

Merged

nolanlawson commented May 16, 2023

View reviewed changes

This was referenced May 17, 2023

NX does not always restore dist files #3511

Closed

chore: run package.json updater when script changed #3516

Merged

fix(wire-service): re-expose wire-service package #3515

Merged

fix: add proxy-compat comment back to dist files #3526

Merged

This was referenced May 26, 2023

2.45.3 @lwc/rollup-plugin breaks Yarn PnP #3540

Closed

fix(rollup-plugin): ensure dependencies are declared #3542

Merged

nolanlawson mentioned this pull request Aug 11, 2023

Normalize the build script across all sub packages of lwc #1558

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor: use consistent build process across packages #3456

refactor: use consistent build process across packages #3456

nolanlawson commented Apr 11, 2023 •

edited

Loading

nolanlawson Apr 11, 2023 •

edited

Loading

nolanlawson Apr 11, 2023

nolanlawson Apr 11, 2023

divmain May 11, 2023

nolanlawson May 11, 2023

nolanlawson Apr 11, 2023

nolanlawson May 9, 2023

nolanlawson Apr 11, 2023 •

edited

Loading

nolanlawson commented Apr 12, 2023 •

edited

Loading

nolanlawson commented Apr 12, 2023 •

edited

Loading

ekashida commented Apr 26, 2023

nolanlawson commented May 1, 2023

nolanlawson commented May 9, 2023

nolanlawson May 9, 2023

divmain left a comment

divmain May 11, 2023

nolanlawson May 11, 2023

divmain May 11, 2023

nolanlawson commented May 11, 2023

ekashida commented May 16, 2023

nolanlawson May 16, 2023

refactor: use consistent build process across packages #3456

refactor: use consistent build process across packages #3456

Conversation

nolanlawson commented Apr 11, 2023 • edited Loading

Details

Does this pull request introduce a breaking change?

Does this pull request introduce an observable change?

nolanlawson Apr 11, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nolanlawson Apr 11, 2023 • edited Loading

Choose a reason for hiding this comment

nolanlawson commented Apr 12, 2023 • edited Loading

nolanlawson commented Apr 12, 2023 • edited Loading

ekashida commented Apr 26, 2023

nolanlawson commented May 1, 2023

nolanlawson commented May 9, 2023

Choose a reason for hiding this comment

divmain left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nolanlawson commented May 11, 2023

ekashida commented May 16, 2023

Choose a reason for hiding this comment

nolanlawson commented Apr 11, 2023 •

edited

Loading

nolanlawson Apr 11, 2023 •

edited

Loading

nolanlawson Apr 11, 2023 •

edited

Loading

nolanlawson commented Apr 12, 2023 •

edited

Loading

nolanlawson commented Apr 12, 2023 •

edited

Loading