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

Addon Dev - Allow ts,gts,gjs files as publicEntrypoints #1106

Merged
merged 1 commit into from
Feb 5, 2022
Merged

Addon Dev - Allow ts,gts,gjs files as publicEntrypoints #1106

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
5 changes: 4 additions & 1 deletion packages/addon-dev/src/rollup-app-reexports.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,10 @@ export default function appReexports(opts: {
let pkg = readJsonSync('package.json');
let appJS: Record<string, string> = {};
for (let filename of Object.keys(bundle)) {
if (opts.include.some((glob) => minimatch(filename, glob))) {
if (
Copy link
Collaborator

Choose a reason for hiding this comment

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

I kinda like the idea of being able to have .d.ts files co-located with the js and js.map files!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Even for the app files that get's merged into the host app? You can still have colocated in the other files, not in the app folder.

Copy link
Collaborator

Choose a reason for hiding this comment

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

oh, I mean I support this change! I'm excited for it!
I've been making separate types directories because of the way it used to be

opts.include.some((glob) => minimatch(filename, glob)) &&
!minimatch(filename, '**/*.d.ts')
) {
appJS[`./${filename}`] = `./dist/_app_/${filename}`;
this.emitFile({
type: 'asset',
Expand Down
6 changes: 5 additions & 1 deletion packages/addon-dev/src/rollup-public-entrypoints.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@ import walkSync from 'walk-sync';
import type { Plugin } from 'rollup';
import { join } from 'path';

function normalizeFileExt(fileName: string) {
return fileName.replace(/\.ts|\.gts|\.gjs$/, '.js');
Copy link
Collaborator

@NullVoxPopuli NullVoxPopuli Feb 4, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

tl;dr: I'm back to thinking this PR is fine, and we don't need changes.
It's a positive improvement and if anything definitive comes up, we can do more PRs

🙃

by adding \.hbs to this list of to-replace extensions, I see that my button.hbs is now included in the app-re-exports:
NullVoxPopuli/ember-addon-v2-typescript-demo@93b85af

This is the exact change to your PR

 function normalizeFileExt(fileName: string) {
-  return fileName.replace(/\.ts|\.gts|\.gjs$/, '.js');
+  return fileName.replace(/\.ts|\.gts|\.gjs|\.hbs$/, '.js');
 }

dist/components/demo/button.js now exists, and looks correct:

// ❯ cat dist/components/demo/button.js 
import { setComponentTemplate } from '@ember/component';
import { hbs } from 'ember-cli-htmlbars';

var FlipButton = setComponentTemplate(FlipButton, hbs`<button {{on 'click' @onClick}}>
  flip
</button>
`);

export { FlipButton as default };

and the demo/index.js in dist looks like:

// ❯ cat dist/components/demo/index.js 
import { setComponentTemplate } from '@ember/component';
import TEMPLATE from './index.js';
import { __decorate } from 'tslib';
import { t as tracked, C as Component } from '../../tracked-b2e133b9.js';
import FlipButton from './button.js';
import 'ember-cli-htmlbars';
import '@glimmer/application';

class Demo extends Component {
  constructor(...args) {
    super(...args);
    this.Button = FlipButton;
    this.active = false;

    this.flip = () => this.active = !this.active;
  }

}

__decorate([tracked], Demo.prototype, "active", void 0);

setComponentTemplate(TEMPLATE, Demo);

export { Demo as default };

which is so close to being correct.
Still has that pesky @glimmer/application import.

Copy link
Collaborator

Choose a reason for hiding this comment

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

import TEMPLATE from './index.js';

this seems like it'll break things though 🤔

Copy link
Collaborator

Choose a reason for hiding this comment

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

sorry for all the noise on the PR -- having a hard time with formatting and nested <details>

For some reason I now have to supply externals, and I think that's breaking a bunch of stuff (lots of stuff to exclude) -- I think addon.dependencies() is supposed to do all this..

In this commit: NullVoxPopuli/ember-addon-v2-typescript-demo@7834ea2

I now get this (with no changes to this PR, #1106):

❯ cat dist/components/demo/index.js 
import { setComponentTemplate } from '@ember/component';
import { hbs } from 'ember-cli-htmlbars';
import { __decorate } from 'tslib';
import Component from '@glimmer/component';
import { tracked } from '@glimmer/tracking';

var TEMPLATE = setComponentTemplate(TEMPLATE, hbs`Hello there!

<out>{{this.active}}</out>

<this.Button @onClick={{this.flip}} />
`);

var FlipButton = setComponentTemplate(FlipButton, hbs`<button {{on 'click' @onClick}}>
  flip
</button>
`);

class Demo extends Component {
  constructor(...args) {
    super(...args);
    this.Button = FlipButton;
    this.active = false;

    this.flip = () => this.active = !this.active;
  }

}

__decorate([tracked], Demo.prototype, "active", void 0);

setComponentTemplate(TEMPLATE, Demo);

export { Demo as default };

}

export default function publicEntrypoints(args: {
srcDir: string;
include: string[];
Expand All @@ -15,7 +19,7 @@ export default function publicEntrypoints(args: {
this.emitFile({
type: 'chunk',
id: join(args.srcDir, name),
fileName: name,
fileName: normalizeFileExt(name),
});
}
},
Expand Down