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

requirejs still a dev dependency in some package.json files #920

Closed
zepumph opened this issue Mar 24, 2020 · 8 comments
Closed

requirejs still a dev dependency in some package.json files #920

zepumph opened this issue Mar 24, 2020 · 8 comments

Comments

@zepumph
Copy link
Member

zepumph commented Mar 24, 2020

I think that universally these shouldn't be needed, but I thought I would check with everyone before deleting.

chipper
installer-builder
phet-io
phet-io-wrappers
studio

Found in the above repos' package.json file.

@chrisklus
Copy link
Contributor

chrisklus commented Apr 2, 2020

From 4/2/20 dev meeting:

Related to #905.
We're happy with this being deleted. Thanks!

@zepumph please assign @Denz1994 to verify that the installer-builder is all good when you're finished.

@pixelzoom
Copy link
Contributor

3 repos (chipper, phet-io-wrappers, studio) have this in package.json. I have no idea whether this is vestigial, is still a dependency, or is a dependency that needs to be removed.

"requirejs": "~2.3.3"

All repos have something like this in package.json. If the namespace is still needed, it should probably be renamed to something other than "requirejsNamespace".

"requirejsNamespace": "ACID_BASE_SOLUTIONS",

@ariel-phet
Copy link
Contributor

@jonathanolson said he can handle this issue (consult with @chrisklus if you have iO related questions).

@ariel-phet
Copy link
Contributor

ariel-phet commented May 11, 2020

Marking as high priority as we would like to clean up ES-6 modules issues in a timely manner

@jonathanolson
Copy link
Contributor

Verified it was OK to remove, and removed in the above commits.

All repos have something like this in package.json. If the namespace is still needed, it should probably be renamed to something other than "requirejsNamespace"

That's the best name we have for it, since it's specifically to work with how we USED to handle string keys. Should that be tagged for dev meeting discussion? I recall others feeling that it wasn't the best name also.

@pixelzoom
Copy link
Contributor

Verified that "requirejs": "~2.3.3" entries were removed.

It doesn't look like "requirejsNamespace" was addressed, see #920 (comment). Back to @jonathanolson.

@jonathanolson
Copy link
Contributor

It doesn't look like "requirejsNamespace" was addressed

Correct, I mentioned:

That's the best name we have for it, since it's specifically to work with how we USED to handle string keys. Should that be tagged for dev meeting discussion? I recall others feeling that it wasn't the best name also.

Proactively tagging this for developer meeting, so we can discuss.

@pixelzoom
Copy link
Contributor

Sorry, I somehow missed that. I'm OK with leaving it as "requirejsNamespace".

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

No branches or pull requests

6 participants