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

Address these issues in new apps (5.9): #1967

Merged

Conversation

NullVoxPopuli
Copy link
Collaborator

@NullVoxPopuli NullVoxPopuli commented Jun 8, 2024

In a new 5.9 app, these deprecations occur

DEPRECATION: The parts property on path nodes is deprecated, use head and tail instead
DEPRECATION: The 'Program' visitor node is deprecated. Use 'Template' or 'Block' instead (node was 'Template')
DEPRECATION: The parts property on path nodes is deprecated, use head and tail instead
DEPRECATION: The 'Program' visitor node is deprecated. Use 'Template' or 'Block' instead (node was 'Template')
DEPRECATION: The 'Program' visitor node is deprecated. Use 'Template' or 'Block' instead (node was 'Template')
DEPRECATION: The 'Program' visitor node is deprecated. Use 'Template' or 'Block' instead (node was 'Template')
DEPRECATION: The 'Program' visitor node is deprecated. Use 'Template' or 'Block' instead (node was 'Template')

The goal of this PR is to get the deprecations down to 0.

The suggested fix is to change the Program visitor to either Template or Block.
and both of those have been around since glimmer-vm 0.39.0, which was: 2019, Jan 18.
after ember-source 3.7 :: 2019, Jan 7
before Octane Preview: ember-source 3.13 :: 2019, Sept 25.
but, 0.39.0's changes weren't pulled in to ember-source until ember-source 3.17............ and using glimmer-vm 0.47.9

Program is more closely analogous to Template which


The node.path.parts vs node.path.head support is easy, because we have something that we can ask if its the new API or not.

@NullVoxPopuli NullVoxPopuli force-pushed the address-deprecation-from-ember-source-5.9-@glimmer/syntax-0.92 branch from 5acf119 to 3b5e573 Compare June 11, 2024 15:20
@ef4
Copy link
Contributor

ef4 commented Jun 11, 2024

I think the remaining failures here are now real ones from this branch.

DEPRECATION: The parts property on path nodes is deprecated, use head and tail instead
DEPRECATION: The 'Program' visitor node is deprecated. Use 'Template' or 'Block' instead (node was 'Template')
DEPRECATION: The parts property on path nodes is deprecated, use head and tail instead
DEPRECATION: The 'Program' visitor node is deprecated. Use 'Template' or 'Block' instead (node was 'Template')
DEPRECATION: The 'Program' visitor node is deprecated. Use 'Template' or 'Block' instead (node was 'Template')
DEPRECATION: The 'Program' visitor node is deprecated. Use 'Template' or 'Block' instead (node was 'Template')
DEPRECATION: The 'Program' visitor node is deprecated. Use 'Template' or 'Block' instead (node was 'Template')
@NullVoxPopuli NullVoxPopuli force-pushed the address-deprecation-from-ember-source-5.9-@glimmer/syntax-0.92 branch from 641cbb9 to fd08c5e Compare June 11, 2024 17:16
@ef4
Copy link
Contributor

ef4 commented Jun 11, 2024

Program wasn't just the root. It also got called for every block, which updated the scopeStack.

@NullVoxPopuli
Copy link
Collaborator Author

Program wasn't just the root. It also got called for every block, which updated the scopeStack.

ah, so Block probably needs to be in the transform as well.

@ef4
Copy link
Contributor

ef4 commented Jun 11, 2024

Suggested solution: #1979

@NullVoxPopuli
Copy link
Collaborator Author

beautiful

@NullVoxPopuli NullVoxPopuli marked this pull request as ready for review June 11, 2024 17:52
@ef4 ef4 merged commit 907bbc6 into stable Jun 11, 2024
201 checks passed
@ef4 ef4 deleted the address-deprecation-from-ember-source-5.9-@glimmer/syntax-0.92 branch June 11, 2024 18:26
@ef4 ef4 added the bug Something isn't working label Jun 11, 2024
@github-actions github-actions bot mentioned this pull request Jun 11, 2024
@johanrd
Copy link
Contributor

johanrd commented Jun 12, 2024

Thanks. Most of the warnings are gone with ember-source: 5.9 + @embroider/compat: 3.5.2 + @embroider/core: 3.4.11

I still get some of these, though:

DEPRECATION: The this property on path nodes is deprecated, use head.type instead
DEPRECATION: The parts property on path nodes is deprecated, use head and tail instead

Total new logged warnings upon build are down ~90% (from 17357 to 2255)

@NullVoxPopuli
Copy link
Collaborator Author

Do you have a repro? in a fresh app I get 0 of those.
You may also want to verify that your lockfile has resolved to the newer @embroider/macros version for each of your dependencies that depends on it

@johanrd
Copy link
Contributor

johanrd commented Jun 13, 2024

Seems like upgrading @embroider/webpack removes the rest of the deprecation warnings for me for some weird reason.

-  "@embroider/webpack": "4.0.3",
+ "@embroider/webpack": "4.0.2",

maybe related to that specific upgrade, but more likely some lockfile accident, as I've also recreated the lockfile and node_modules.

@mkszepp
Copy link
Contributor

mkszepp commented Jun 24, 2024

@NullVoxPopuli @ef4 we still get this deprecation warnings with ember 5.9 (also 5.8) & latest embroider release (all packages in app are up to date, except eslint as v9 is still not supported in eslint-plugin-ember).

I hope its not caused by ember-moment because this package is the only one which still doesn't ship embroider v1 😒
https://github.com/adopted-ember-addons/ember-moment/blob/b3ad8be3b6b3bda4eae226dcc16d6cdb2e95d81d/package.json#L42-L44

My PR was never accepted adopted-ember-addons/ember-moment#378

There is installed only @embroider/macros v1.16.4

@mkszepp
Copy link
Contributor

mkszepp commented Jun 24, 2024

looks like there was any embroider cache in my case... now error doesn't appear anymore

@shama
Copy link

shama commented Jul 3, 2024

We are still hitting some deprecations messages in our build with the latest dependencies. I pushed a repro app here: https://github.com/shama/embroider-deprecation-repro

I haven't had a chance to look where the deprecation messages originate from. It only happens on first the first build but not subsequent builds unless you clear out the embroider tmp folder.

In this repo, run npm run clean to remove the tmp folder and then npm run build to repro.

@ef4
Copy link
Contributor

ef4 commented Jul 3, 2024

It looks like all the remaining deprecations in there are coming from embroider's resolver transform:

let rootName = node.path.parts[0];

@ef4
Copy link
Contributor

ef4 commented Jul 3, 2024

It would be a helpful PR if somebody wants to go through that file (against the embroider stable branch) and update all of the .parts and .this` to the non-deprecated alternatives.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants