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.

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

Inplement runtime portion of RFC #931 #20776

Merged
merged 28 commits into from
Nov 6, 2024
Merged

Inplement runtime portion of RFC #931 #20776

merged 28 commits into from
Nov 6, 2024

Conversation

wycats
Copy link
Member

@wycats wycats commented Oct 10, 2024

No description provided.

This commit fixes shadowing bugs in multiple AST transforms
This should be handled by a linter so it can be disabled where appropriate.
1. Document the type signature of the `template()` options.
2. Consolidate plugins into `@ember/template-compiler` and import them
   from `ember-template-compiler`.

This commit also creates a few new entry points in
`@ember/template-compiler` to satisfy the use-cases in the
consolidation.
@wycats wycats marked this pull request as ready for review October 16, 2024 15:01
@wycats wycats changed the title Draft implementation of RFC #931 Inplement runtime portion of RFC #931 Oct 16, 2024
This time in eval, where all the tooling in the world didn't notice it.
1. Re-export all of the plugins from locations in
  `ember-template-compiler/lib/plugins` to support the AMD compat
  entrypoints. I prioritized avoiding changes to the
  `amd-compat-entrypoints` file over absolutely minimizing the amount of
  duplication in `ember-template-compiler`. The implementations
  themselves are still not duplicated, but if we create new files in
  `@ember/template-compiler`, this may have implications for
  `ember-template-compiler`.
2. Fix a problem where `export import` in the barrel file wasn't playing
   nice with some piece of updated infrastructure.
3. Rename some of the exposed exports to include `-internal` in their
   module name.
This time, use an ambient `declare namespace` to declare the
`Ember.RSVP` types, which will avoid the ill-fated attempt to transpile
it at runtime using the babel typescript transform. The previous
implementation already required us to dynamically install the `RSVP`
property on the `Ember` object because the previous transpiler
infrastructure didn't reliably handle this situation, so this is
probably an improvement either way.
@NullVoxPopuli
Copy link
Contributor

NullVoxPopuli commented Oct 18, 2024

Safari 17.6 passes locally for me.
I see that browserstack is using Safari 12 (we did for 5.x).
We don't actually support that.

According to: https://browsersl.ist/#q=%3E+0.25%25+and+not+dead
per: https://emberjs.com/browser-support/

I think we can bump to Safari 15.6

PR: #20779

NullVoxPopuli added a commit to NullVoxPopuli/ember-source-pr-20776-testing that referenced this pull request Oct 18, 2024
@@ -292,6 +292,30 @@
"@ember/runloop/index.js": "ember-source/@ember/runloop/index.js",
"@ember/service/index.js": "ember-source/@ember/service/index.js",
"@ember/template-compilation/index.js": "ember-source/@ember/template-compilation/index.js",
"@ember/template-compiler/-internal-primitives.js": "ember-source/@ember/template-compiler/-internal-primitives.js",
Copy link
Contributor

Choose a reason for hiding this comment

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

just tested and this new stuff is not available outside of embroider 👍

@NullVoxPopuli
Copy link
Contributor

I tried this out in an embroider app here:
https://github.com/NullVoxPopuli/ember-source-pr-20776-testing

and it didn't work (seems like nothing is rendered?).

maybe whatever is going on with CI will be resolved when this reproduction is resolved?

@NullVoxPopuli
Copy link
Contributor

NullVoxPopuli commented Oct 21, 2024

Looks like there are two copies of @glimmer/manager now, which breaks setComponentTemplate.

Supposedly, I have sourcemaps turned off
image

@ef4
Copy link
Contributor

ef4 commented Oct 21, 2024

Your repro link is broken and this is not enough information to say what's going wrong. There are known module-duplication bugs that effect ember-source that are fixed only in the very latest main of embroider.

@NullVoxPopuli
Copy link
Contributor

Your repro link is broken and this is not enough information to say what's going wrong.

I don't understand, the link works.
Also, all templates are just.. not rendered (which I said 🤔 ).

Unless you mean something more specific?

There are known module-duplication bugs that effect ember-source that are fixed only in the very latest main of embroider.

in any case, I suppose for this feature we don't want to fix for embroider3? which means we have to disable/skip the failing test?

@NullVoxPopuli
Copy link
Contributor

NullVoxPopuli commented Oct 21, 2024

I confirm things work as expected in the vite blueprint:

image
code here: https://github.com/NullVoxPopuli/ember-source-pr-20776-testing-vite

What not-working looks like in these tests
image
code here: https://github.com/NullVoxPopuli/ember-source-pr-20776-testing

Copy link
Contributor

@NullVoxPopuli NullVoxPopuli left a comment

Choose a reason for hiding this comment

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

All green!

@ef4
Copy link
Contributor

ef4 commented Oct 23, 2024

I merged #20782 so this needs conflict fixes.

@ef4 ef4 merged commit 988dd79 into main Nov 6, 2024
24 checks passed
@ef4 ef4 deleted the feature/rfc-0931 branch November 6, 2024 14:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants