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

Remove unneeded babel plugins so that shipped addon code is way smaller and easier to debug without sourcemaps #231

Merged
merged 9 commits into from
Dec 8, 2023
+8 −17
Merged

Remove unneeded babel plugins so that shipped addon code is way smaller and easier to debug without sourcemaps #231

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
5b9ab33
Try decorator transforms
NullVoxPopuli Nov 28, 2023
b714fdd
Remove babel plugins
NullVoxPopuli Nov 28, 2023
ac76bdc
lockfile
NullVoxPopuli Nov 28, 2023
da98807
We can keep json babel config
NullVoxPopuli Nov 28, 2023
48e33d4
json should have .json 😅
NullVoxPopuli Nov 28, 2023
63dbb3f
Update babel.config.json
NullVoxPopuli Dec 8, 2023
9df70d1
Update babel.config.json
NullVoxPopuli Dec 8, 2023
f1c53de
reducelock
NullVoxPopuli Dec 8, 2023
5410e19
Update tests
NullVoxPopuli Dec 8, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 1 addition & 3 deletions files/__addonLocation__/babel.config.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,10 @@
"plugins": [
<% if (typescript) { %> ["@babel/plugin-transform-typescript", { "allExtensions": true, "onlyRemoveTypeImports": true, "allowDeclareFields": true }],
<% } %> "@embroider/addon-dev/template-colocation-plugin",
"@babel/plugin-transform-class-static-block",
["babel-plugin-ember-template-compilation", {
"targetFormat": "hbs",
"transforms": []
}],
["@babel/plugin-proposal-decorators", { "version": "legacy" }],
"@babel/plugin-proposal-class-properties"
["module:decorator-transforms", { "runtime": { "import": "decorator-transforms/runtime" } }],
]
}
6 changes: 2 additions & 4 deletions files/__addonLocation__/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,14 +33,12 @@
"prepack": "rollup --config"
},
"dependencies": {
"@embroider/addon-shim": "^1.0.0"
"@embroider/addon-shim": "^1.0.0",
"decorator-transforms": "^1.0.1"
Copy link
Contributor

@SergeAstapov SergeAstapov Dec 8, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

@NullVoxPopuli out of curiosity why would it go to dependencies? Is it intentional or mistake?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

because it has a tiny runtime module that needs to be imported

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

https://github.com/ef4/decorator-transforms/blob/main/src/runtime.ts

},
"devDependencies": {
"@babel/core": "^7.17.0",
<% if (typescript) { %>"@babel/plugin-transform-typescript": "^7.22.15"<% } else { %>"@babel/eslint-parser": "^7.19.1"<% } %>,
"@babel/plugin-proposal-class-properties": "^7.16.7",
"@babel/plugin-proposal-decorators": "^7.20.13",
"@babel/plugin-transform-class-static-block": "^7.20.0",
"@babel/runtime": "^7.17.0",
"@embroider/addon-dev": "^4.1.0",<% if (typescript) { %>
"@glint/core": "^1.2.0",
Expand Down
6 changes: 3 additions & 3 deletions tests/smoke-tests/--addon-only.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ describe('--addon-only', () => {
expect(rootContents).to.include('.editorconfig');
expect(rootContents).to.include('.eslintignore');
expect(rootContents).to.include('.eslintrc.cjs');
expect(rootContents).to.include('.gitignore');
expect(rootContents).to.include('.gitignore');
expect(rootContents).to.include('.npmrc');
expect(rootContents).to.include('.prettierignore');
expect(rootContents).to.include('.prettierrc.cjs');
Expand All @@ -39,11 +39,11 @@ describe('--addon-only', () => {
expect(rootContents).to.include('rollup.config.mjs');

let configContents = await dirContents(path.join(helper.projectRoot, 'config'));

expect(configContents).to.include('ember-cli-update.json');

let srcContents = await dirContents(path.join(helper.projectRoot, 'src'));

expect(srcContents).to.include('index.js');
});

Expand Down
9 changes: 2 additions & 7 deletions tests/smoke-tests/--typescript.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,17 +89,12 @@ for (let packageManager of SUPPORTED_PACKAGE_MANAGERS) {
'template-registry.js.map',
]);

expect(distContents.hashed.length).toBe(4);
expect(
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

we no longer emit rollup helpers

distContents.hashed
.filter((file) => file.includes('_rollup'))
.map((file) => file.split('.js')[1]),
).to.deep.equal(['', '.map'], 'the rollup helpers are emitted with a source map');
expect(distContents.hashed.length).toBe(2);
expect(
distContents.hashed
.filter((file) => file.includes('template-only'))
.map((file) => file.split('.js')[1]),
).to.deep.equal(['', '.map'], 'the template-only component is emitted with a source map');
).to.deep.equal(['', '.map'], 'the template-only (private) component is emitted with a source map');
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

just clarifying here that this assertion is ensuring that private components are still present in the output


expect(declarationsContents).to.deep.equal([
'components',
Expand Down