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

Advance RFC #0939 "Introduce a Wildcard Module Import API" to Stage Ready for Release #988

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

emberjs-rfcs-bot
Copy link
Collaborator

Advance #939 to the Ready For Release Stage

Summary

This pull request is advancing the RFC to the Ready For Release Stage.

An FCP is required before merging this PR to advance.

Upon merging this PR, automation will open a draft PR for this RFC to move to the Released Stage.

Ready for Release Stage Description

This stage is complete when the implementation is complete according to plan outlined in the RFC, and is in harmony with any changes in Ember that have occurred since the RFC was first written. This includes any necessary learning materials. At this stage, features or deprecations may be available for use behind a feature flag, or with an optional package, etc.

For codebase changes, there are no open questions that are anticipated to require breaking changes; the Ember team is ready to commit to the stability of any interfaces exposed by the current implementation of the feature.

This stage should include a list of criteria for determining when the proposal can be considered Recommended after being Released.

An FCP is required to move into this stage.

Each Ember core team will be requested as a reviewer on the PR to move into this stage. A representative of each team adds a review. If a team does not respond to the request, and after the conclusion of the FCP, it is assumed that the release may proceed.

Checklist to move to Ready for Release

  • Implementation is complete according to plan outlined in the RFC, with any adjustments noted in the RFC
  • Any necessary learning materials have been updated
  • The Ember team is ready to commit to the stability of any interfaces exposed by the current implementation of the feature. This is the go/no go decision for any feature flags, but the flags should only be turned on when moving to Released.
  • Criteria for moving to the Recommended Stage has been filled out
  • This PR has been converted from a draft to a regular PR and the Final Comment Period label has been added to start the FCP
  • Each team has been added as a reviewer to the PR at the start of the FCP. Reviews are not required by the end of the FCP. This is a notification step.
    • Framework @emberjs/framework
    • Data @emberjs/ember-data-core
    • CLI @emberjs/cli
    • Learning @emberjs/learning-core
    • Typescript @emberjs/typescript-core
    • Steering @emberjs/steering

Criteria for moving to Recommended (required)

A set of criteria for moving this RFC to the Recommended Stage, following release:

Track Implementation

<-- Use this section to track implementation of the RFC -->

@emberjs-rfcs-bot emberjs-rfcs-bot added RFC Advancement S-Ready for Release PR to move to the Ready for Release Stage labels Nov 10, 2023
@ef4 ef4 self-assigned this Dec 1, 2023
@ef4 ef4 assigned ef4 and mansona and unassigned ef4 and mansona Jan 30, 2024
@ef4
Copy link
Contributor

ef4 commented Sep 20, 2024

@ef4
Copy link
Contributor

ef4 commented Oct 15, 2024

Other implementation progress: https://github.com/beerinho/ember-glob-import-demo

@NullVoxPopuli
Copy link
Contributor

NullVoxPopuli commented Nov 20, 2024

Some issues I've found with the build-time only implementation (at least for the classic / non-appImports usage),

Other implementation progress: https://github.com/beerinho/ember-glob-import-demo

but as I've been playing around with the implementation (in a local copy of it), I've been tweaking things, making notes, etc.

  • ../ isn't supported (useful for going from the tests directory to the app directory
    • work-around: have a top-level app/whatever.js that you import from tests and app/whatever.js does the import.meta.glob
  • ./ doesn't work as expected from within sub-folders because in classic requirejs requires the full moduleName and relative requires aren't a thing
    • fixing this would require either deriving the moduleName from files babel doesn't normally care about, or requiring a config passed to the babel plugin
  • I added a extra ignore entry for .d.ts files, since that cannot ever make it in to an output file

Implementation progress: https://github.com/NullVoxPopuli/ember-classic-import-meta-glob

  • supports ../
  • supports ./
  • doesn't deal with .d.ts

however, this does not mean we shouldn't continue pursing the build-time-only approach, as these are all solveable problems with v1 addons

  • use v1 addon infra to:
    • get the moduleName
      • allows solving ../
    • install the babel plugin

A helpful package here is: path-normalize, which collapses ../ and ./: https://www.npmjs.com/package/path-normalize?activeTab=readme

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
RFC Advancement S-Ready for Release PR to move to the Ready for Release Stage
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants