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

refactor: migrate on acorn-import-attributes #18251

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

alexander-akait
Copy link
Member

What kind of change does this PR introduce?

xtuc/acorn-import-attributes#31

Did you add tests for your changes?

Existing

Does this PR introduce a breaking change?

No

What needs to be documented once your changes are merged?

Nothing

@webpack-bot
Copy link
Contributor

For maintainers only:

  • This needs to be documented (issue in webpack/webpack.js.org will be filed when merged)
  • This needs to be backported to webpack 4 (issue will be created when merged)

@alexander-akait
Copy link
Member Author

Need to fix xtuc/acorn-import-attributes#31 (comment)

@ahabhgk
Copy link
Contributor

ahabhgk commented Apr 3, 2024

Can I take this? 😁

@alexander-akait
Copy link
Member Author

@ahabhgk I am already working on it 😄 we have small problems here, need to deprecate the assert rule in favor attributes, on the other hand, it might be worth preserving both - assert and attributes, so developers can write different rules for differents imports, i.e. import json from './foo.json' assert { type: 'json' }; and import json from './foo.json' with { type: 'json' };, not sure how better, what do you think?

@ahabhgk
Copy link
Contributor

ahabhgk commented Apr 3, 2024

https://twitter.com/NicoloRibaudo/status/1772721841670402384

My opinion is deprecate the assert rule in favor attributes (or with?) rule, and remove the assert rule in the next major, since all mainstream tools and platforms will remove it

@alexander-akait
Copy link
Member Author

@ahabhgk Yeah, let's do it, I think attributes is better here, because in future we can have more attributes, not only with and the spec has name import-attributes 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants